From 59c7f801f6dfbd6bc7095d7b071bf124bbf81538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 13:26:49 +0100 Subject: [PATCH 01/30] refactor(v2fix): enhance KnownChange interface with Clone method and improve context handling This is needed to fix the race condition that exists on concurrent evaluations. --- tools/v2fix/v2fix/known_change.go | 39 ++++++++++++++++++++++++++++++- tools/v2fix/v2fix/v2fix.go | 7 ++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 6433646383..2f5c77a25b 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -43,6 +43,9 @@ type KnownChange interface { // SetNode updates the node with the given value. SetNode(ast.Node) + + // Clone creates a fresh copy of this KnownChange for thread-safe concurrent use. + Clone() KnownChange } type contextHandler struct { @@ -96,6 +99,9 @@ func (d defaultKnownChange) Pos() token.Pos { } func eval(k KnownChange, n ast.Node, pass *analysis.Pass) bool { + // Reset context for each node evaluation to prevent data races + // when multiple goroutines analyze different packages concurrently. + k.SetContext(context.Background()) for _, p := range k.Probes() { ctx, ok := p(k.Context(), n, pass) if !ok { @@ -111,6 +117,10 @@ type V1ImportURL struct { defaultKnownChange } +func (V1ImportURL) Clone() KnownChange { + return &V1ImportURL{} +} + func (c V1ImportURL) Fixes() []analysis.SuggestedFix { path := c.ctx.Value(pkgPathKey).(string) if path == "" { @@ -146,6 +156,10 @@ type DDTraceTypes struct { defaultKnownChange } +func (DDTraceTypes) Clone() KnownChange { + return &DDTraceTypes{} +} + func (c DDTraceTypes) Fixes() []analysis.SuggestedFix { typ, ok := c.ctx.Value(declaredTypeKey).(*types.Named) if !ok { @@ -193,6 +207,10 @@ type TracerStructs struct { defaultKnownChange } +func (TracerStructs) Clone() KnownChange { + return &TracerStructs{} +} + func (c TracerStructs) Fixes() []analysis.SuggestedFix { typ, ok := c.ctx.Value(declaredTypeKey).(*types.Named) if !ok { @@ -234,6 +252,10 @@ type WithServiceName struct { defaultKnownChange } +func (WithServiceName) Clone() KnownChange { + return &WithServiceName{} +} + func (c WithServiceName) Fixes() []analysis.SuggestedFix { args, ok := c.ctx.Value(argsKey).([]ast.Expr) if !ok || args == nil { @@ -274,6 +296,10 @@ type TraceIDString struct { defaultKnownChange } +func (TraceIDString) Clone() KnownChange { + return &TraceIDString{} +} + func (c TraceIDString) Fixes() []analysis.SuggestedFix { fn, ok := c.ctx.Value(fnKey).(*types.Func) if !ok || fn == nil { @@ -314,6 +340,10 @@ type WithDogstatsdAddr struct { defaultKnownChange } +func (WithDogstatsdAddr) Clone() KnownChange { + return &WithDogstatsdAddr{} +} + func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { args, ok := c.ctx.Value(argsKey).([]ast.Expr) if !ok || args == nil { @@ -356,6 +386,10 @@ type DeprecatedSamplingRules struct { defaultKnownChange } +func (DeprecatedSamplingRules) Clone() KnownChange { + return &DeprecatedSamplingRules{} +} + func (c DeprecatedSamplingRules) Probes() []Probe { return []Probe{ IsFuncCall, @@ -459,7 +493,10 @@ func (c DeprecatedSamplingRules) String() string { func exprListString(exprs []ast.Expr) string { var buf bytes.Buffer - for _, expr := range exprs { + for i, expr := range exprs { + if i > 0 { + buf.WriteString(", ") + } buf.WriteString(exprString(expr)) } return buf.String() diff --git a/tools/v2fix/v2fix/v2fix.go b/tools/v2fix/v2fix/v2fix.go index 96645e5009..5d8be7d940 100644 --- a/tools/v2fix/v2fix/v2fix.go +++ b/tools/v2fix/v2fix/v2fix.go @@ -50,8 +50,11 @@ func (c Checker) runner() func(*analysis.Pass) (interface{}, error) { ins.Preorder(filter, func(n ast.Node) { var k KnownChange for _, c := range knownChanges { - if eval(c, n, pass) { - k = c + // Clone the KnownChange to avoid data races when multiple + // goroutines analyze different packages concurrently. + clone := c.Clone() + if eval(clone, n, pass) { + k = clone break } } From 75644d613f70a4d2c3791d74f91d33f0831bfce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:13:46 +0100 Subject: [PATCH 02/30] feat(v2fix): implement golden file generation for analyzer suggested fixes This update introduces functionality to generate and update golden files based on suggested fixes from the analyzer. It processes diagnostics, collects edits, and formats the output accordingly, ensuring proper handling of single and multiple messages. This enhancement improves the testing workflow for the v2fix package. The warning-only golden files are generated for consistency/documentation but aren't validated by analysistest.RunWithSuggestedFixes. This is expected behavior from the analysistest package - it only compares when there are actual text edits to apply. --- tools/v2fix/v2fix/golden_generator.go | 264 ++++++++++++++++++++++++++ tools/v2fix/v2fix/v2fix_test.go | 7 + 2 files changed, 271 insertions(+) create mode 100644 tools/v2fix/v2fix/golden_generator.go diff --git a/tools/v2fix/v2fix/golden_generator.go b/tools/v2fix/v2fix/golden_generator.go new file mode 100644 index 0000000000..7e7f004712 --- /dev/null +++ b/tools/v2fix/v2fix/golden_generator.go @@ -0,0 +1,264 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package v2fix + +import ( + "bytes" + "fmt" + "go/format" + "os" + "path/filepath" + "sort" + "testing" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/analysistest" +) + +// diff.Edit from golang.org/x/tools/internal/diff +// We define our own copy since the internal package is not accessible. +type diffEdit struct { + Start, End int + New string +} + +// applyEdits applies a sequence of edits to src and returns the result. +// Edits are applied in order of start position; edits with the same start +// position are applied in the order they appear in the slice. +func applyEdits(src []byte, edits []diffEdit) ([]byte, error) { + if len(edits) == 0 { + return src, nil + } + + // Sort edits by start position + sorted := make([]diffEdit, len(edits)) + copy(sorted, edits) + sort.SliceStable(sorted, func(i, j int) bool { + if sorted[i].Start != sorted[j].Start { + return sorted[i].Start < sorted[j].Start + } + return sorted[i].End < sorted[j].End + }) + + var out bytes.Buffer + pos := 0 + for _, edit := range sorted { + if edit.Start < pos { + return nil, fmt.Errorf("overlapping edit at position %d (current pos: %d)", edit.Start, pos) + } + if edit.Start > len(src) || edit.End > len(src) { + return nil, fmt.Errorf("edit out of bounds: start=%d end=%d len=%d", edit.Start, edit.End, len(src)) + } + // Copy bytes before this edit + out.Write(src[pos:edit.Start]) + // Write the new text + out.WriteString(edit.New) + pos = edit.End + } + // Copy remaining bytes + out.Write(src[pos:]) + + return out.Bytes(), nil +} + +// runWithSuggestedFixesUpdate runs the analyzer and generates/updates golden files +// instead of comparing against them. +func runWithSuggestedFixesUpdate(t *testing.T, dir string, a *analysis.Analyzer, patterns ...string) { + t.Helper() + + // Run the analyzer using the standard analysistest.Run + results := analysistest.Run(t, dir, a, patterns...) + + // Process results to generate golden files + for _, result := range results { + act := result.Action + + // Collect edits per file and per message + // file path -> message -> edits + fileEdits := make(map[string]map[string][]diffEdit) + fileContents := make(map[string][]byte) + + for _, diag := range act.Diagnostics { + for _, fix := range diag.SuggestedFixes { + for _, edit := range fix.TextEdits { + start, end := edit.Pos, edit.End + if !end.IsValid() { + end = start + } + + file := act.Package.Fset.File(start) + if file == nil { + continue + } + + fileName := file.Name() + if _, ok := fileContents[fileName]; !ok { + contents, err := os.ReadFile(fileName) + if err != nil { + t.Errorf("error reading %s: %v", fileName, err) + continue + } + fileContents[fileName] = contents + } + + if _, ok := fileEdits[fileName]; !ok { + fileEdits[fileName] = make(map[string][]diffEdit) + } + + fileEdits[fileName][fix.Message] = append( + fileEdits[fileName][fix.Message], + diffEdit{ + Start: file.Offset(start), + End: file.Offset(end), + New: string(edit.NewText), + }, + ) + } + } + } + + // Generate golden files + for fileName, fixes := range fileEdits { + orig := fileContents[fileName] + goldenPath := fileName + ".golden" + + // Check if we have multiple different messages (requires txtar format) + // or a single message (simpler format) + messages := make([]string, 0, len(fixes)) + for msg := range fixes { + messages = append(messages, msg) + } + sort.Strings(messages) + + var golden bytes.Buffer + + if len(messages) == 1 { + // Single message: use simple txtar format with one section + msg := messages[0] + edits := fixes[msg] + + out, err := applyEdits(orig, edits) + if err != nil { + t.Errorf("error applying edits to %s: %v", fileName, err) + continue + } + + formatted, err := format.Source(out) + if err != nil { + // If formatting fails, use unformatted + formatted = out + } + + golden.WriteString("-- ") + golden.WriteString(msg) + golden.WriteString(" --\n") + golden.Write(formatted) + } else { + // Multiple messages: create txtar archive with multiple sections + for _, msg := range messages { + edits := fixes[msg] + + out, err := applyEdits(orig, edits) + if err != nil { + t.Errorf("error applying edits to %s for message %q: %v", fileName, msg, err) + continue + } + + formatted, err := format.Source(out) + if err != nil { + formatted = out + } + + golden.WriteString("-- ") + golden.WriteString(msg) + golden.WriteString(" --\n") + golden.Write(formatted) + golden.WriteString("\n") + } + } + + // Ensure the golden content ends with a newline + content := golden.Bytes() + if len(content) > 0 && content[len(content)-1] != '\n' { + content = append(content, '\n') + } + + if err := os.WriteFile(goldenPath, content, 0644); err != nil { + t.Errorf("error writing golden file %s: %v", goldenPath, err) + continue + } + + // Get relative path for cleaner output + relPath := goldenPath + if rel, err := filepath.Rel(dir, goldenPath); err == nil { + relPath = rel + } + t.Logf("Updated golden file: %s", relPath) + } + + // Handle files that have diagnostics but no suggested fixes + // (e.g., warnings without auto-fix) + for _, diag := range act.Diagnostics { + if len(diag.SuggestedFixes) == 0 { + // Find the file for this diagnostic + file := act.Package.Fset.File(diag.Pos) + if file == nil { + continue + } + + fileName := file.Name() + // Skip if we already processed this file with fixes + if _, ok := fileEdits[fileName]; ok { + continue + } + + // Read file contents if not already cached + if _, ok := fileContents[fileName]; !ok { + contents, err := os.ReadFile(fileName) + if err != nil { + continue + } + fileContents[fileName] = contents + } + + goldenPath := fileName + ".golden" + + // Check if golden file already exists - don't overwrite + // if there's nothing to fix + if _, err := os.Stat(goldenPath); err == nil { + // Golden file exists, skip + continue + } + + // Create a golden file with just the message header and original content + var golden bytes.Buffer + golden.WriteString("-- ") + golden.WriteString(diag.Message) + golden.WriteString(" --\n") + golden.Write(fileContents[fileName]) + + content := golden.Bytes() + if len(content) > 0 && content[len(content)-1] != '\n' { + content = append(content, '\n') + } + + if err := os.WriteFile(goldenPath, content, 0644); err != nil { + t.Errorf("error writing golden file %s: %v", goldenPath, err) + continue + } + + relPath := goldenPath + if rel, err := filepath.Rel(dir, goldenPath); err == nil { + relPath = rel + } + t.Logf("Updated golden file (no fixes): %s", relPath) + + // Mark as processed + fileEdits[fileName] = make(map[string][]diffEdit) + } + } + } +} diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index 758f45c651..8e352fdf9a 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -7,6 +7,7 @@ package v2fix import ( "context" + "flag" "fmt" "go/ast" "go/types" @@ -18,6 +19,8 @@ import ( "golang.org/x/tools/go/analysis/analysistest" ) +var update = flag.Bool("update", false, "update golden files") + type V1Usage struct { ctx context.Context } @@ -101,6 +104,10 @@ func testRunner(t *testing.T, name string) func(*analysis.Analyzer) { return nil } return func(a *analysis.Analyzer) { + if *update { + runWithSuggestedFixesUpdate(t, path.Join(cwd, "..", "_stage"), a, fmt.Sprintf("./%s", name)) + return + } analysistest.RunWithSuggestedFixes(t, path.Join(cwd, "..", "_stage"), a, fmt.Sprintf("./%s", name)) } } From e7eb887f446fe9bc27a03f86d5d33b5330ce1d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:16:36 +0100 Subject: [PATCH 03/30] test(v2fix): add V1Usage Clone stub for tests The tests require V1Usage to satisfy the KnownChange interface, and adding Clone() to the test helper makes that contract explicit so test scaffolding stays aligned with production interface changes without altering runtime behavior. --- tools/v2fix/v2fix/v2fix_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index 8e352fdf9a..db2dd353e6 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -25,6 +25,10 @@ type V1Usage struct { ctx context.Context } +func (V1Usage) Clone() KnownChange { + return &V1Usage{} +} + func (c V1Usage) Context() context.Context { return c.ctx } From ea0f22729671c5f89ecd27e52433b6880a17230d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:21:32 +0100 Subject: [PATCH 04/30] feat(v2fix): enhance probes with type alias/composite handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This updates probe context plumbing to capture type expressions, import literals, and helper logic for aliases, composite types, and v1-path detection so analysis is more accurate (including ChildOf option extraction) and fixes can target precise source spans without misidentifying non‑v1 symbols. --- tools/v2fix/v2fix/context.go | 25 +- tools/v2fix/v2fix/known_change.go | 7 + tools/v2fix/v2fix/probe.go | 446 ++++++++++++++++++++++++++++-- 3 files changed, 447 insertions(+), 31 deletions(-) diff --git a/tools/v2fix/v2fix/context.go b/tools/v2fix/v2fix/context.go index 99a74f150b..382021b0f2 100644 --- a/tools/v2fix/v2fix/context.go +++ b/tools/v2fix/v2fix/context.go @@ -8,14 +8,19 @@ package v2fix type contextKey string const ( - argsKey contextKey = "args" - declaredTypeKey contextKey = "declared_type" - endKey contextKey = "end" - fnKey contextKey = "fn" - pkgNameKey contextKey = "pkg_name" - pkgPrefixKey contextKey = "pkg_prefix" - pkgPathKey contextKey = "pkg_path" - posKey contextKey = "pos" - typeKey contextKey = "type" - callExprKey contextKey = "call_expr" + argsKey contextKey = "args" + declaredTypeKey contextKey = "declared_type" + endKey contextKey = "end" + fnKey contextKey = "fn" + pkgNameKey contextKey = "pkg_name" + pkgPrefixKey contextKey = "pkg_prefix" + pkgPathKey contextKey = "pkg_path" + posKey contextKey = "pos" + typeKey contextKey = "type" + callExprKey contextKey = "call_expr" + typeExprStrKey contextKey = "type_expr_str" + typePrefixKey contextKey = "type_prefix" // Stores modifiers like "*", "[]", "[N]" for composite types + skipFixKey contextKey = "skip_fix" // Set to true when a fix cannot be safely applied + childOfParentKey contextKey = "childof_parent" // Stores the parent expression string for ChildOf transformation + childOfOtherOptsKey contextKey = "childof_other" // Stores other options (excluding ChildOf) for StartSpan ) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 2f5c77a25b..0899a35423 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -98,6 +98,13 @@ func (d defaultKnownChange) Pos() token.Pos { return d.node.Pos() } +func (d defaultKnownChange) pkgPrefix() string { + if pkg, ok := d.ctx.Value(pkgPrefixKey).(string); ok && pkg != "" { + return pkg + } + return "tracer" +} + func eval(k KnownChange, n ast.Node, pass *analysis.Pass) bool { // Reset context for each node evaluation to prevent data races // when multiple goroutines analyze different packages concurrently. diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index 7bbaed2629..d053ed20af 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -6,10 +6,13 @@ package v2fix import ( + "bytes" "context" "go/ast" + "go/printer" "go/types" "reflect" + "strconv" "strings" "golang.org/x/tools/go/analysis" @@ -18,15 +21,28 @@ import ( type Probe func(context.Context, ast.Node, *analysis.Pass) (context.Context, bool) +// getTypeNameFromType extracts the TypeName object from Named or Alias types. +func getTypeNameFromType(t types.Type) *types.TypeName { + switch t := t.(type) { + case *types.Named: + return t.Obj() + case *types.Alias: + return t.Obj() + } + return nil +} + // DeclaresType returns true if the node declares a type of the given generic type. // The type use in the generic signature is stored in the context as "type". // The reflected type is stored in the context as "declared_type". +// The formatted type expression string is stored as "type_expr_str". +// Handles both *types.Named and *types.Alias (Go 1.22+ type aliases). func DeclaresType[T any]() Probe { return func(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { var ( - obj types.Object typ = ctx.Value(typeKey) typDecl ast.Expr + varType types.Type ) switch typ { case "*ast.ValueSpec": @@ -34,41 +50,63 @@ func DeclaresType[T any]() Probe { if len(spec.Names) == 0 { return ctx, false } - obj = pass.TypesInfo.ObjectOf(spec.Names[0]) typDecl = spec.Type + // Try to get type from object first (works for named vars) + obj := pass.TypesInfo.ObjectOf(spec.Names[0]) + if obj != nil { + varType = obj.Type() + } else if typDecl != nil { + // For blank identifiers, get type from the type expression. + varType = getTypeFromTypeExpr(typDecl, pass) + } case "*ast.Field": field := n.(*ast.Field) if len(field.Names) == 0 { return ctx, false } - obj = pass.TypesInfo.ObjectOf(field.Names[0]) typDecl = field.Type + obj := pass.TypesInfo.ObjectOf(field.Names[0]) + if obj != nil { + varType = obj.Type() + } else if typDecl != nil { + varType = getTypeFromTypeExpr(typDecl, pass) + } default: return ctx, false } if typDecl == nil { return ctx, false } - t, ok := obj.Type().(*types.Named) - if !ok { + if varType == nil { + return ctx, false + } + + // Extract type object info, handling both *types.Named and *types.Alias + typeObj := getTypeNameFromType(varType) + if typeObj == nil { return ctx, false } - // We need to store the reflected type unconditionally - // to be able to introspect it later, even if the probe - // fails or is combined with Not. - ctx = context.WithValue(ctx, declaredTypeKey, t) + ctx = context.WithValue(ctx, declaredTypeKey, varType) + ctx = context.WithValue(ctx, posKey, typDecl.Pos()) ctx = context.WithValue(ctx, endKey, typDecl.End()) + + // Store formatted type expression string to preserve original qualifier/alias + var buf bytes.Buffer + if err := printer.Fprint(&buf, pass.Fset, typDecl); err == nil { + ctx = context.WithValue(ctx, typeExprStrKey, buf.String()) + } + v := new(T) e := reflect.TypeOf(v).Elem() - if t.Obj().Pkg() == nil { + if typeObj.Pkg() == nil { return ctx, false } - ctx = context.WithValue(ctx, pkgNameKey, t.Obj().Pkg().Name()) - if t.Obj().Pkg().Path() != e.PkgPath() { + ctx = context.WithValue(ctx, pkgNameKey, typeObj.Pkg().Name()) + if typeObj.Pkg().Path() != e.PkgPath() { return ctx, false } - if t.Obj().Name() != e.Name() { + if typeObj.Name() != e.Name() { return ctx, false } return ctx, true @@ -129,13 +167,21 @@ func IsFuncCall(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.C // IsImport returns true if the node is an import statement. // The import path is stored in the context as "pkg_path". +// The pos/end keys are set to the import path literal position (not the alias). func IsImport(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { imp, ok := n.(*ast.ImportSpec) if !ok { return ctx, false } - path := strings.Trim(imp.Path.Value, `"`) + // Use strconv.Unquote to properly handle both regular and raw string imports + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + return ctx, false + } ctx = context.WithValue(ctx, pkgPathKey, path) + // Set pos/end to the import path literal position so V1ImportURL edits only the string literal + ctx = context.WithValue(ctx, posKey, imp.Path.Pos()) + ctx = context.WithValue(ctx, endKey, imp.Path.End()) return ctx, true } @@ -152,11 +198,16 @@ func HasPackagePrefix(prefix string) Probe { } // ImportedFrom returns true if the value is imported from the given package path prefix. +// It checks both the resolved type's package path AND the AST import path (for type aliases). +// It also sets declaredTypeKey in the context when a named type is found. +// Handles composite types (pointers, slices, arrays) by unwrapping to the base type +// and storing the type prefix (e.g., "*", "[]") in typePrefixKey. func ImportedFrom(pkgPath string) Probe { return func(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { var ( - obj types.Object - typ = ctx.Value(typeKey) + typ = ctx.Value(typeKey) + varType types.Type + typDecl ast.Expr ) switch typ { case "*ast.ValueSpec": @@ -164,24 +215,229 @@ func ImportedFrom(pkgPath string) Probe { if len(spec.Names) == 0 { return ctx, false } - obj = pass.TypesInfo.ObjectOf(spec.Names[0]) + typDecl = spec.Type + obj := pass.TypesInfo.ObjectOf(spec.Names[0]) + if obj != nil { + varType = obj.Type() + } else if typDecl != nil { + varType = getTypeFromTypeExpr(typDecl, pass) + } case "*ast.Field": field := n.(*ast.Field) if len(field.Names) == 0 { return ctx, false } - obj = pass.TypesInfo.ObjectOf(field.Names[0]) + typDecl = field.Type + obj := pass.TypesInfo.ObjectOf(field.Names[0]) + if obj != nil { + varType = obj.Type() + } else if typDecl != nil { + varType = getTypeFromTypeExpr(typDecl, pass) + } + default: + return ctx, false + } + + // Set pos/end to the type expression position for accurate fix targeting + if typDecl != nil { + ctx = context.WithValue(ctx, posKey, typDecl.Pos()) + ctx = context.WithValue(ctx, endKey, typDecl.End()) + } + + // Unwrap composite types (pointer, slice, array) to get the base type + // and store the prefix for use in fixes + var typePrefix string + var prefixValid bool + baseTypDecl := typDecl + if typDecl != nil { + baseTypDecl, typePrefix, prefixValid = unwrapTypeExpr(typDecl) + if typePrefix != "" { + if !prefixValid { + // Array length couldn't be rendered; skip this fix to avoid + // corrupting the type (e.g., turning [N+1]T into []T) + ctx = context.WithValue(ctx, skipFixKey, true) + } + ctx = context.WithValue(ctx, typePrefixKey, typePrefix) + // Also get the base type from the unwrapped expression + varType = getTypeFromTypeExpr(baseTypDecl, pass) + } + } + + // Store the resolved type in context for use by later probes + // Support both *types.Named and *types.Alias (Go 1.22+) + if varType != nil && getTypeNameFromType(varType) != nil { + ctx = context.WithValue(ctx, declaredTypeKey, varType) + } + + // For type aliases, the resolved type may be from a different package. + // Check the AST type expression's import path first (more reliable for aliases). + // Use the unwrapped base type expression for the lookup. + if baseTypDecl != nil { + if importPath := getImportPathFromTypeExpr(baseTypDecl, pass, n); importPath != "" { + if strings.HasPrefix(importPath, pkgPath) { + return ctx, true + } + } + } + + // Then check the resolved type's package path + if t, ok := varType.(*types.Named); ok { + if pkg := t.Obj().Pkg(); pkg != nil && strings.HasPrefix(pkg.Path(), pkgPath) { + return ctx, true + } + } + return ctx, false + } +} + +// unwrapTypeExpr unwraps pointer, slice, and array type expressions to get the base type. +// It returns the base type expression, a prefix string (e.g., "*", "[]", "[N]") to prepend, +// and a boolean indicating whether the prefix is valid (false if array length couldn't be safely rendered). +// When prefixValid is false, the prefix contains "[?]" as a placeholder and the fix should be skipped, +// but the diagnostic should still be emitted. +func unwrapTypeExpr(typDecl ast.Expr) (ast.Expr, string, bool) { + var prefix strings.Builder + valid := true + for { + switch t := typDecl.(type) { + case *ast.StarExpr: + prefix.WriteByte('*') + typDecl = t.X + case *ast.ArrayType: + if t.Len == nil { + // Slice type - safe to render + prefix.WriteString("[]") + } else if lit, isLit := t.Len.(*ast.BasicLit); isLit { + // Literal array length (e.g., [5]) - safe to include in fix + prefix.WriteByte('[') + prefix.WriteString(lit.Value) + prefix.WriteByte(']') + } else { + // Non-literal array length (identifier, expression, etc.) + // Skip fix to preserve original formatting, but continue to detect type + prefix.WriteString("[?]") + valid = false + } + typDecl = t.Elt default: + return typDecl, prefix.String(), valid + } + } +} + +// getTypeFromTypeExpr extracts the type from a type expression. +// This handles various cases including blank identifiers and type aliases. +func getTypeFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass) types.Type { + // Try TypeOf first (works for value expressions) + if t := pass.TypesInfo.TypeOf(typDecl); t != nil { + return t + } + // Try Types map (works for type expressions) + if tv, ok := pass.TypesInfo.Types[typDecl]; ok && tv.Type != nil { + return tv.Type + } + // For SelectorExpr like pkg.Type, look up the type directly + if sel, ok := typDecl.(*ast.SelectorExpr); ok { + // Look up the selector (the type name) in Uses + if obj := pass.TypesInfo.Uses[sel.Sel]; obj != nil { + if tn, ok := obj.(*types.TypeName); ok { + return tn.Type() + } + } + // Fallback: check ObjectOf + if obj := pass.TypesInfo.ObjectOf(sel.Sel); obj != nil { + if tn, ok := obj.(*types.TypeName); ok { + return tn.Type() + } + } + } + return nil +} + +// getImportPathFromTypeExpr extracts the import path from a type expression like "pkg.Type". +// It looks up the package identifier in pass.TypesInfo.Uses to find the imported package. +func getImportPathFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass, n ast.Node) string { + sel, ok := typDecl.(*ast.SelectorExpr) + if !ok { + return "" + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return "" + } + // Look up the identifier to find the package it refers to. + // Uses maps identifiers to the objects they denote. + if obj, ok := pass.TypesInfo.Uses[ident]; ok { + if pkgName, ok := obj.(*types.PkgName); ok { + return pkgName.Imported().Path() + } + } + // Fallback: for package identifiers, ObjectOf might work + if obj := pass.TypesInfo.ObjectOf(ident); obj != nil { + if pkgName, ok := obj.(*types.PkgName); ok { + return pkgName.Imported().Path() + } + } + // Last resort: search the current file's imports for a matching name. + // Find the file that contains this node. + nodePos := n.Pos() + for _, file := range pass.Files { + if file.Pos() <= nodePos && nodePos < file.End() { + for _, imp := range file.Imports { + name := "" + if imp.Name != nil { + name = imp.Name.Name + } else { + // Use the last part of the path as the default name + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + continue + } + parts := strings.Split(path, "/") + name = parts[len(parts)-1] + } + if name == ident.Name { + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + continue + } + return path + } + } + break // Found the file, no need to continue + } + } + return "" +} + +// HasBaseType returns true if the declared base type (after unwrapping composite types) +// matches the given generic type T. This is useful for checking types wrapped in +// pointers, slices, or arrays (e.g., *T, []T, [N]T). +// It expects declaredTypeKey to be set by ImportedFrom (which stores the unwrapped type). +func HasBaseType[T any]() Probe { + return func(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { + declaredType := ctx.Value(declaredTypeKey) + if declaredType == nil { return ctx, false } - t, ok := obj.Type().(*types.Named) + varType, ok := declaredType.(types.Type) if !ok { return ctx, false } - if t.Obj().Pkg() == nil { + typeObj := getTypeNameFromType(varType) + if typeObj == nil { + return ctx, false + } + if typeObj.Pkg() == nil { + return ctx, false + } + + v := new(T) + e := reflect.TypeOf(v).Elem() + if typeObj.Pkg().Path() != e.PkgPath() { return ctx, false } - if !strings.HasPrefix(t.Obj().Pkg().Path(), pkgPath) { + if typeObj.Name() != e.Name() { return ctx, false } return ctx, true @@ -218,3 +474,151 @@ func Or(ps ...Probe) Probe { return ctx, false } } + +// HasV1PackagePath returns true if the function's package path starts with the v1 prefix. +// This is used to reduce false positives by ensuring we only match dd-trace-go v1 packages. +// It expects the pkgPathKey to be set by IsFuncCall. +func HasV1PackagePath(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { + pkgPath, ok := ctx.Value(pkgPathKey).(string) + if !ok { + return ctx, false + } + return ctx, strings.HasPrefix(pkgPath, "gopkg.in/DataDog/dd-trace-go.v1") +} + +// IsV1Import returns true if the import path is a v1 dd-trace-go import. +// Matches both the root import "gopkg.in/DataDog/dd-trace-go.v1" and subpath imports. +// It expects pkgPathKey to be set by IsImport. +func IsV1Import(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { + pkgPath, ok := ctx.Value(pkgPathKey).(string) + if !ok { + return ctx, false + } + const v1Root = "gopkg.in/DataDog/dd-trace-go.v1" + // Match exact root or subpath (with trailing slash) + return ctx, pkgPath == v1Root || strings.HasPrefix(pkgPath, v1Root+"/") +} + +// HasChildOfOption returns true if the StartSpan call has a ChildOf option. +// It extracts the parent expression and stores it in childOfParentKey. +// Other options (excluding ChildOf) are stored in childOfOtherOptsKey. +func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { + args, ok := ctx.Value(argsKey).([]ast.Expr) + if !ok || len(args) < 2 { + return ctx, false + } + callExpr, _ := ctx.Value(callExprKey).(*ast.CallExpr) + hasEllipsis := callExpr != nil && callExpr.Ellipsis.IsValid() + + var parentExpr string + var otherOpts []string + foundChildOf := false + + isChildOfCall := func(arg ast.Expr) bool { + call, ok := arg.(*ast.CallExpr) + if !ok { + return false + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + return sel.Sel.Name == "ChildOf" + } + + // Check all args after the first one (operation name) for ChildOf calls + for _, arg := range args[1:] { + call, ok := arg.(*ast.CallExpr) + if !ok { + if opt := exprToString(arg); opt != "" { + otherOpts = append(otherOpts, opt) + } + continue + } + + // Check if this is a ChildOf call + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + if opt := exprToString(arg); opt != "" { + otherOpts = append(otherOpts, opt) + } + continue + } + + if sel.Sel.Name == "ChildOf" { + foundChildOf = true + // Extract the parent expression from ChildOf(parent.Context()) or ChildOf(parentCtx) + if len(call.Args) > 0 { + parentArg := call.Args[0] + // Check if it's a parent.Context() call - we want to use just "parent" + if callExpr, ok := parentArg.(*ast.CallExpr); ok { + if selExpr, ok := callExpr.Fun.(*ast.SelectorExpr); ok { + if selExpr.Sel.Name == "Context" { + parentExpr = exprToString(selExpr.X) + continue + } + } + } + // Otherwise use the full expression + parentExpr = exprToString(parentArg) + } + } else { + // This is not ChildOf, collect it as another option + if opt := exprToString(arg); opt != "" { + otherOpts = append(otherOpts, opt) + } + } + } + + if !foundChildOf || parentExpr == "" { + return ctx, false + } + // Preserve ellipsis on the last argument if present. + if hasEllipsis { + lastArg := args[len(args)-1] + if isChildOfCall(lastArg) { + // Cannot preserve ellipsis if it applies to ChildOf itself. + return ctx, false + } + if len(otherOpts) == 0 { + return ctx, false + } + otherOpts[len(otherOpts)-1] = otherOpts[len(otherOpts)-1] + "..." + } + + ctx = context.WithValue(ctx, childOfParentKey, parentExpr) + ctx = context.WithValue(ctx, childOfOtherOptsKey, otherOpts) + return ctx, true +} + +// exprToString converts an AST expression to a string representation. +// This is a simplified version that handles common cases. +func exprToString(expr ast.Expr) string { + switch e := expr.(type) { + case *ast.Ident: + return e.Name + case *ast.SelectorExpr: + return exprToString(e.X) + "." + e.Sel.Name + case *ast.CallExpr: + return exprToString(e.Fun) + "(" + exprListToString(e.Args) + ")" + case *ast.BasicLit: + return e.Value + case *ast.IndexExpr: + return exprToString(e.X) + "[" + exprToString(e.Index) + "]" + case *ast.StarExpr: + return "*" + exprToString(e.X) + case *ast.UnaryExpr: + return e.Op.String() + exprToString(e.X) + case *ast.ParenExpr: + return "(" + exprToString(e.X) + ")" + } + return "" +} + +func exprListToString(exprs []ast.Expr) string { + var parts []string + for _, e := range exprs { + parts = append(parts, exprToString(e)) + } + return strings.Join(parts, ", ") +} From 6a741f9e0fe7688650d718321766cc3864575122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:23:44 +0100 Subject: [PATCH 05/30] feat(v2fix): map v1 contrib imports to v2 layout This extends the v1 import rewrite to recognize contrib subpaths and translate them to the new contrib/.../v2 path while still handling core imports, ensuring fixes produce correct module paths for both patterns. --- tools/v2fix/v2fix/known_change.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 0899a35423..5c1f103c44 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -133,7 +133,19 @@ func (c V1ImportURL) Fixes() []analysis.SuggestedFix { if path == "" { return nil } - path = strings.Replace(path, "gopkg.in/DataDog/dd-trace-go.v1", "github.com/DataDog/dd-trace-go/v2", 1) + + const v1Prefix = "gopkg.in/DataDog/dd-trace-go.v1" + const contribPrefix = v1Prefix + "/contrib/" + + if strings.HasPrefix(path, contribPrefix) { + // Contrib imports: gopkg.in/DataDog/dd-trace-go.v1/contrib/X → github.com/DataDog/dd-trace-go/contrib/X/v2 + contribPath := strings.TrimPrefix(path, contribPrefix) + path = "github.com/DataDog/dd-trace-go/contrib/" + contribPath + "/v2" + } else { + // Core imports: gopkg.in/DataDog/dd-trace-go.v1/X → github.com/DataDog/dd-trace-go/v2/X + path = strings.Replace(path, v1Prefix, "github.com/DataDog/dd-trace-go/v2", 1) + } + return []analysis.SuggestedFix{ { Message: "update import URL to v2", @@ -151,7 +163,7 @@ func (c V1ImportURL) Fixes() []analysis.SuggestedFix { func (V1ImportURL) Probes() []Probe { return []Probe{ IsImport, - HasPackagePrefix("gopkg.in/DataDog/dd-trace-go.v1/"), + IsV1Import, } } From efb0c1ba34927f989a8e0d23aae76cce65fa8a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:28:14 +0100 Subject: [PATCH 06/30] feat(v2fix): handle renamed AppSec login events Adds the AppSecLoginEvents known change plus wiring in main and tests so v2fix can remap TrackUserLogin(Success|Failure)Event to the new TrackUserLogin(Success|Failure) APIs and keep the golden fixtures in sync; this keeps the AppSec migration path covered and avoids false positives against the renamed helpers. --- tools/v2fix/_stage/appseclogin/appseclogin.go | 22 +++++++ .../_stage/appseclogin/appseclogin.go.golden | 23 +++++++ tools/v2fix/main.go | 1 + tools/v2fix/v2fix/known_change.go | 63 +++++++++++++++++++ tools/v2fix/v2fix/v2fix_test.go | 5 ++ 5 files changed, 114 insertions(+) create mode 100644 tools/v2fix/_stage/appseclogin/appseclogin.go create mode 100644 tools/v2fix/_stage/appseclogin/appseclogin.go.golden diff --git a/tools/v2fix/_stage/appseclogin/appseclogin.go b/tools/v2fix/_stage/appseclogin/appseclogin.go new file mode 100644 index 0000000000..8f17eae5f7 --- /dev/null +++ b/tools/v2fix/_stage/appseclogin/appseclogin.go @@ -0,0 +1,22 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "context" + + "gopkg.in/DataDog/dd-trace-go.v1/appsec" +) + +func main() { + ctx := context.Background() + + // Login success event + appsec.TrackUserLoginSuccessEvent(ctx, "user123", nil) // want `appsec login event functions have been renamed` + + // Login failure event + appsec.TrackUserLoginFailureEvent(ctx, "user123", false, nil) // want `appsec login event functions have been renamed` +} diff --git a/tools/v2fix/_stage/appseclogin/appseclogin.go.golden b/tools/v2fix/_stage/appseclogin/appseclogin.go.golden new file mode 100644 index 0000000000..2fca9a61d6 --- /dev/null +++ b/tools/v2fix/_stage/appseclogin/appseclogin.go.golden @@ -0,0 +1,23 @@ +-- appsec login event functions have been renamed (remove 'Event' suffix) -- +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "context" + + "gopkg.in/DataDog/dd-trace-go.v1/appsec" +) + +func main() { + ctx := context.Background() + + // Login success event + appsec.TrackUserLoginSuccess(ctx, "user123", nil) // want `appsec login event functions have been renamed` + + // Login failure event + appsec.TrackUserLoginFailure(ctx, "user123", false, nil) // want `appsec login event functions have been renamed` +} diff --git a/tools/v2fix/main.go b/tools/v2fix/main.go index d8645cf087..58205141ab 100644 --- a/tools/v2fix/main.go +++ b/tools/v2fix/main.go @@ -19,6 +19,7 @@ func main() { &v2fix.WithServiceName{}, &v2fix.WithDogstatsdAddr{}, &v2fix.DeprecatedSamplingRules{}, + &v2fix.AppSecLoginEvents{}, ) c.Run(singlechecker.Main) } diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 5c1f103c44..a8928c31de 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -552,3 +552,66 @@ func exprString(expr ast.Expr) string { } return "" } + +// AppSecLoginEvents handles the renaming of appsec login event functions. +// TrackUserLoginSuccessEvent → TrackUserLoginSuccess +// TrackUserLoginFailureEvent → TrackUserLoginFailure +type AppSecLoginEvents struct { + defaultKnownChange +} + +func (AppSecLoginEvents) Clone() KnownChange { + return &AppSecLoginEvents{} +} + +func (c AppSecLoginEvents) Probes() []Probe { + return []Probe{ + IsFuncCall, + HasV1PackagePath, + Or( + WithFunctionName("TrackUserLoginSuccessEvent"), + WithFunctionName("TrackUserLoginFailureEvent"), + ), + } +} + +func (c AppSecLoginEvents) Fixes() []analysis.SuggestedFix { + fn, ok := c.ctx.Value(fnKey).(*types.Func) + if !ok || fn == nil { + return nil + } + + args, ok := c.ctx.Value(argsKey).([]ast.Expr) + if !ok { + return nil + } + + pkg := c.pkgPrefix() + var newFuncName string + switch fn.Name() { + case "TrackUserLoginSuccessEvent": + newFuncName = "TrackUserLoginSuccess" + case "TrackUserLoginFailureEvent": + newFuncName = "TrackUserLoginFailure" + default: + return nil + } + + newText := fmt.Sprintf("%s.%s(%s)", pkg, newFuncName, exprListString(args)) + return []analysis.SuggestedFix{ + { + Message: "appsec login event functions have been renamed (remove 'Event' suffix)", + TextEdits: []analysis.TextEdit{ + { + Pos: c.Pos(), + End: c.End(), + NewText: []byte(newText), + }, + }, + }, + } +} + +func (c AppSecLoginEvents) String() string { + return "appsec login event functions have been renamed (remove 'Event' suffix)" +} diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index db2dd353e6..cb5ce2c317 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -100,6 +100,11 @@ func TestDeprecatedSamplingRules(t *testing.T) { c.Run(testRunner(t, "samplingrules")) } +func TestAppSecLoginEvents(t *testing.T) { + c := NewChecker(&AppSecLoginEvents{}) + c.Run(testRunner(t, "appseclogin")) +} + func testRunner(t *testing.T, name string) func(*analysis.Analyzer) { t.Helper() cwd, err := os.Getwd() From 9073652dece68e39b4e86dc96ab108c4839bbdd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:34:20 +0100 Subject: [PATCH 07/30] feat(v2fix): warn when WithPrioritySampling is still referenced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the DeprecatedWithPrioritySampling KnownChange plus new golden fixture/output to cover tracer.WithPrioritySampling so the checker can emit the “has been removed” warning (priority sampling is now default) without attempting any code changes, keeping migrations focused on the new semantics. --- .../withprioritysampling.go | 16 +++++++++++ .../withprioritysampling.go.golden | 17 +++++++++++ tools/v2fix/main.go | 1 + tools/v2fix/v2fix/known_change.go | 28 +++++++++++++++++++ tools/v2fix/v2fix/v2fix_test.go | 5 ++++ 5 files changed, 67 insertions(+) create mode 100644 tools/v2fix/_stage/withprioritysampling/withprioritysampling.go create mode 100644 tools/v2fix/_stage/withprioritysampling/withprioritysampling.go.golden diff --git a/tools/v2fix/_stage/withprioritysampling/withprioritysampling.go b/tools/v2fix/_stage/withprioritysampling/withprioritysampling.go new file mode 100644 index 0000000000..8c527c4737 --- /dev/null +++ b/tools/v2fix/_stage/withprioritysampling/withprioritysampling.go @@ -0,0 +1,16 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + // WithPrioritySampling is removed - priority sampling is now default + tracer.Start(tracer.WithPrioritySampling()) // want `WithPrioritySampling has been removed` + defer tracer.Stop() +} diff --git a/tools/v2fix/_stage/withprioritysampling/withprioritysampling.go.golden b/tools/v2fix/_stage/withprioritysampling/withprioritysampling.go.golden new file mode 100644 index 0000000000..1727c4d3b9 --- /dev/null +++ b/tools/v2fix/_stage/withprioritysampling/withprioritysampling.go.golden @@ -0,0 +1,17 @@ +-- WithPrioritySampling has been removed; priority sampling is now enabled by default -- +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + // WithPrioritySampling is removed - priority sampling is now default + tracer.Start(tracer.WithPrioritySampling()) // want `WithPrioritySampling has been removed` + defer tracer.Stop() +} diff --git a/tools/v2fix/main.go b/tools/v2fix/main.go index 58205141ab..70d5ebef45 100644 --- a/tools/v2fix/main.go +++ b/tools/v2fix/main.go @@ -20,6 +20,7 @@ func main() { &v2fix.WithDogstatsdAddr{}, &v2fix.DeprecatedSamplingRules{}, &v2fix.AppSecLoginEvents{}, + &v2fix.DeprecatedWithPrioritySampling{}, ) c.Run(singlechecker.Main) } diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index a8928c31de..b6594d6821 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -615,3 +615,31 @@ func (c AppSecLoginEvents) Fixes() []analysis.SuggestedFix { func (c AppSecLoginEvents) String() string { return "appsec login event functions have been renamed (remove 'Event' suffix)" } + +// DeprecatedWithPrioritySampling warns about usage of WithPrioritySampling which has been removed. +// Priority sampling is now enabled by default. +type DeprecatedWithPrioritySampling struct { + defaultKnownChange +} + +func (DeprecatedWithPrioritySampling) Clone() KnownChange { + return &DeprecatedWithPrioritySampling{} +} + +func (c DeprecatedWithPrioritySampling) Probes() []Probe { + return []Probe{ + IsFuncCall, + HasV1PackagePath, + WithFunctionName("WithPrioritySampling"), + } +} + +func (c DeprecatedWithPrioritySampling) Fixes() []analysis.SuggestedFix { + // Warning only - no auto-fix since it should just be removed + return nil +} + +func (c DeprecatedWithPrioritySampling) String() string { + return "WithPrioritySampling has been removed; priority sampling is now enabled by default" +} + diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index cb5ce2c317..bf7d0a22f6 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -105,6 +105,11 @@ func TestAppSecLoginEvents(t *testing.T) { c.Run(testRunner(t, "appseclogin")) } +func TestDeprecatedWithPrioritySampling(t *testing.T) { + c := NewChecker(&DeprecatedWithPrioritySampling{}) + c.Run(testRunner(t, "withprioritysampling")) +} + func testRunner(t *testing.T, name string) func(*analysis.Analyzer) { t.Helper() cwd, err := os.Getwd() From 93de85cd1d615e3e4c460c39bbd89be75ca985d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:36:15 +0100 Subject: [PATCH 08/30] feat(v2fix): warn on removed WithHTTPRoundTripper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the DeprecatedWithHTTPRoundTripper KnownChange, the associated golden fixture inputs/outputs, and registers it so v2fix can detect tracer.WithHTTPRoundTripper calls and emit the “has been removed; use WithHTTPClient instead” warning; this keeps the migration checker aware of the removed option instead of letting outdated code pass silently. --- .../withhttproundtripper.go | 16 ++++++++++++ .../withhttproundtripper.go.golden | 17 +++++++++++++ tools/v2fix/main.go | 1 + tools/v2fix/v2fix/known_change.go | 25 +++++++++++++++++++ tools/v2fix/v2fix/v2fix_test.go | 6 +++++ 5 files changed, 65 insertions(+) create mode 100644 tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go create mode 100644 tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go.golden diff --git a/tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go b/tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go new file mode 100644 index 0000000000..8bdeb3db4d --- /dev/null +++ b/tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go @@ -0,0 +1,16 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + // WithHTTPRoundTripper is removed - use WithHTTPClient instead + tracer.Start(tracer.WithHTTPRoundTripper(nil)) // want `WithHTTPRoundTripper has been removed` + defer tracer.Stop() +} diff --git a/tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go.golden b/tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go.golden new file mode 100644 index 0000000000..97417dae76 --- /dev/null +++ b/tools/v2fix/_stage/withhttproundtripper/withhttproundtripper.go.golden @@ -0,0 +1,17 @@ +-- WithHTTPRoundTripper has been removed; use WithHTTPClient instead -- +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + // WithHTTPRoundTripper is removed - use WithHTTPClient instead + tracer.Start(tracer.WithHTTPRoundTripper(nil)) // want `WithHTTPRoundTripper has been removed` + defer tracer.Stop() +} diff --git a/tools/v2fix/main.go b/tools/v2fix/main.go index 70d5ebef45..7c81b15495 100644 --- a/tools/v2fix/main.go +++ b/tools/v2fix/main.go @@ -21,6 +21,7 @@ func main() { &v2fix.DeprecatedSamplingRules{}, &v2fix.AppSecLoginEvents{}, &v2fix.DeprecatedWithPrioritySampling{}, + &v2fix.DeprecatedWithHTTPRoundTripper{}, ) c.Run(singlechecker.Main) } diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index b6594d6821..dc77b8a7d6 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -643,3 +643,28 @@ func (c DeprecatedWithPrioritySampling) String() string { return "WithPrioritySampling has been removed; priority sampling is now enabled by default" } +// DeprecatedWithHTTPRoundTripper warns about usage of WithHTTPRoundTripper which has been removed. +type DeprecatedWithHTTPRoundTripper struct { + defaultKnownChange +} + +func (DeprecatedWithHTTPRoundTripper) Clone() KnownChange { + return &DeprecatedWithHTTPRoundTripper{} +} + +func (c DeprecatedWithHTTPRoundTripper) Probes() []Probe { + return []Probe{ + IsFuncCall, + HasV1PackagePath, + WithFunctionName("WithHTTPRoundTripper"), + } +} + +func (c DeprecatedWithHTTPRoundTripper) Fixes() []analysis.SuggestedFix { + // Warning only - cannot auto-fix since the API signature changed (RoundTripper vs Client) + return nil +} + +func (c DeprecatedWithHTTPRoundTripper) String() string { + return "WithHTTPRoundTripper has been removed; use WithHTTPClient instead" +} diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index bf7d0a22f6..ea6f27222d 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -110,6 +110,12 @@ func TestDeprecatedWithPrioritySampling(t *testing.T) { c.Run(testRunner(t, "withprioritysampling")) } +func TestDeprecatedWithHTTPRoundTripper(t *testing.T) { + c := NewChecker(&DeprecatedWithHTTPRoundTripper{}) + c.Run(testRunner(t, "withhttproundtripper")) +} + + func testRunner(t *testing.T, name string) func(*analysis.Analyzer) { t.Helper() cwd, err := os.Getwd() From cfb8cdc56c1afc3a07b426dee17eecebd19cf173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:39:21 +0100 Subject: [PATCH 09/30] test(v2fix): add false-positive guard coverage Added a falsepositive fixture and regression test that runs the WithServiceName, TraceID, WithDogstatsdAddress, and sampling rule rewrites against locally defined functions/types so that the checker proves it only flags dd-trace-go v1 symbols, preventing the migration tool from warning about unrelated code that happens to share names. --- .../_stage/falsepositive/falsepositive.go | 40 +++++++++++++++++++ tools/v2fix/v2fix/v2fix_test.go | 17 ++++++++ 2 files changed, 57 insertions(+) create mode 100644 tools/v2fix/_stage/falsepositive/falsepositive.go diff --git a/tools/v2fix/_stage/falsepositive/falsepositive.go b/tools/v2fix/_stage/falsepositive/falsepositive.go new file mode 100644 index 0000000000..142b3e5ddd --- /dev/null +++ b/tools/v2fix/_stage/falsepositive/falsepositive.go @@ -0,0 +1,40 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2023 Datadog, Inc. + +// This test verifies that functions with the same names as dd-trace-go v1 functions +// but from different packages are NOT flagged for migration. +package main + +// Local type and function definitions that shadow dd-trace-go names +type Span struct { + Name string +} + +func WithServiceName(name string) string { + return name +} + +func TraceID() uint64 { + return 0 +} + +func WithDogstatsdAddress(addr string) string { + return addr +} + +func ServiceRule(service string, rate float64) string { + return service +} + +func main() { + // These should NOT be flagged - they're local functions, not from dd-trace-go v1 + _ = WithServiceName("test") + _ = TraceID() + _ = WithDogstatsdAddress("localhost:8125") + _ = ServiceRule("myservice", 0.5) + + // Local type should NOT be flagged + var _ Span +} diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index ea6f27222d..640bf46279 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -115,6 +115,23 @@ func TestDeprecatedWithHTTPRoundTripper(t *testing.T) { c.Run(testRunner(t, "withhttproundtripper")) } +// TestFalsePositives verifies that functions with the same names as dd-trace-go v1 functions +// but from different packages are NOT flagged for migration. +func TestFalsePositives(t *testing.T) { + // Test all function-call based changes against the false positive test file + changes := []KnownChange{ + &WithServiceName{}, + &TraceIDString{}, + &WithDogstatsdAddr{}, + &DeprecatedSamplingRules{}, + } + for _, change := range changes { + t.Run(fmt.Sprintf("%T", change), func(t *testing.T) { + c := NewChecker(change) + c.Run(testRunner(t, "falsepositive")) + }) + } +} func testRunner(t *testing.T, name string) func(*analysis.Analyzer) { t.Helper() From 5be42c80a52b223a45a2f226ef57b15548bc3b83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:26:16 +0100 Subject: [PATCH 10/30] =?UTF-8?q?feat(v2fix):=20add=20ChildOf=20=E2=86=92?= =?UTF-8?q?=20StartChild=20rewrite?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the ChildOfStartChild known change along with its probe/test wiring so v2fix can rewrite tracer.StartSpan(..., tracer.ChildOf(...)) into the new parent.StartChild(...) pattern, keeping option plumbing and test coverage aligned with the migration guidance. --- tools/v2fix/_stage/childof/childof.go | 39 ++++++++++++ tools/v2fix/_stage/childof/childof.go.golden | 40 +++++++++++++ tools/v2fix/main.go | 1 + tools/v2fix/v2fix/known_change.go | 62 ++++++++++++++++++++ tools/v2fix/v2fix/v2fix_test.go | 5 ++ 5 files changed, 147 insertions(+) create mode 100644 tools/v2fix/_stage/childof/childof.go create mode 100644 tools/v2fix/_stage/childof/childof.go.golden diff --git a/tools/v2fix/_stage/childof/childof.go b/tools/v2fix/_stage/childof/childof.go new file mode 100644 index 0000000000..d9e6144b50 --- /dev/null +++ b/tools/v2fix/_stage/childof/childof.go @@ -0,0 +1,39 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + tracer.Start() + defer tracer.Stop() + + parent := tracer.StartSpan("parent") + defer parent.Finish() + + extraOpt := tracer.ResourceName("resource") + + // Simple ChildOf usage + child := tracer.StartSpan("child", tracer.ChildOf(parent.Context())) // want `use StartChild instead of StartSpan with ChildOf` + defer child.Finish() + + // ChildOf with additional options + child2 := tracer.StartSpan("child2", tracer.ChildOf(parent.Context()), tracer.ResourceName("resource")) // want `use StartChild instead of StartSpan with ChildOf` + defer child2.Finish() + + // ChildOf with non-call option (variable) + child3 := tracer.StartSpan("child3", tracer.ChildOf(parent.Context()), extraOpt) // want `use StartChild instead of StartSpan with ChildOf` + defer child3.Finish() + + // ChildOf passed via variadic (ellipsis applies to ChildOf itself) - should not be rewritten + parentOpts := []tracer.StartSpanOption{ + tracer.ChildOf(parent.Context()), + } + child4 := tracer.StartSpan("child4", parentOpts...) + defer child4.Finish() +} diff --git a/tools/v2fix/_stage/childof/childof.go.golden b/tools/v2fix/_stage/childof/childof.go.golden new file mode 100644 index 0000000000..70f0f8a4d2 --- /dev/null +++ b/tools/v2fix/_stage/childof/childof.go.golden @@ -0,0 +1,40 @@ +-- use StartChild instead of StartSpan with ChildOf -- +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package main + +import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + tracer.Start() + defer tracer.Stop() + + parent := tracer.StartSpan("parent") + defer parent.Finish() + + extraOpt := tracer.ResourceName("resource") + + // Simple ChildOf usage + child := parent.StartChild("child") // want `use StartChild instead of StartSpan with ChildOf` + defer child.Finish() + + // ChildOf with additional options + child2 := parent.StartChild("child2", tracer.ResourceName("resource")) // want `use StartChild instead of StartSpan with ChildOf` + defer child2.Finish() + + // ChildOf with non-call option (variable) + child3 := parent.StartChild("child3", extraOpt) // want `use StartChild instead of StartSpan with ChildOf` + defer child3.Finish() + + // ChildOf passed via variadic (ellipsis applies to ChildOf itself) - should not be rewritten + parentOpts := []tracer.StartSpanOption{ + tracer.ChildOf(parent.Context()), + } + child4 := tracer.StartSpan("child4", parentOpts...) + defer child4.Finish() +} diff --git a/tools/v2fix/main.go b/tools/v2fix/main.go index 7c81b15495..6c031a1993 100644 --- a/tools/v2fix/main.go +++ b/tools/v2fix/main.go @@ -19,6 +19,7 @@ func main() { &v2fix.WithServiceName{}, &v2fix.WithDogstatsdAddr{}, &v2fix.DeprecatedSamplingRules{}, + &v2fix.ChildOfStartChild{}, &v2fix.AppSecLoginEvents{}, &v2fix.DeprecatedWithPrioritySampling{}, &v2fix.DeprecatedWithHTTPRoundTripper{}, diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index dc77b8a7d6..289849cfd2 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -553,6 +553,68 @@ func exprString(expr ast.Expr) string { return "" } +// ChildOfStartChild handles the transformation of tracer.StartSpan("op", tracer.ChildOf(parent.Context())) +// to parent.StartChild("op"). This is a complex structural change. +type ChildOfStartChild struct { + defaultKnownChange +} + +func (ChildOfStartChild) Clone() KnownChange { + return &ChildOfStartChild{} +} + +func (c ChildOfStartChild) Probes() []Probe { + return []Probe{ + IsFuncCall, + HasV1PackagePath, + WithFunctionName("StartSpan"), + HasChildOfOption, + } +} + +func (c ChildOfStartChild) Fixes() []analysis.SuggestedFix { + args, ok := c.ctx.Value(argsKey).([]ast.Expr) + if !ok || len(args) < 2 { + return nil + } + + // First arg is the operation name + opName := args[0] + + // Get the parent expression from context (set by HasChildOfOption) + parentExpr, ok := c.ctx.Value(childOfParentKey).(string) + if !ok || parentExpr == "" { + return nil + } + + // Get the other options (excluding ChildOf) from context + otherOpts, _ := c.ctx.Value(childOfOtherOptsKey).([]string) + + var newText string + if len(otherOpts) > 0 { + newText = fmt.Sprintf("%s.StartChild(%s, %s)", parentExpr, exprString(opName), strings.Join(otherOpts, ", ")) + } else { + newText = fmt.Sprintf("%s.StartChild(%s)", parentExpr, exprString(opName)) + } + + return []analysis.SuggestedFix{ + { + Message: "use StartChild instead of StartSpan with ChildOf", + TextEdits: []analysis.TextEdit{ + { + Pos: c.Pos(), + End: c.End(), + NewText: []byte(newText), + }, + }, + }, + } +} + +func (c ChildOfStartChild) String() string { + return "use StartChild instead of StartSpan with ChildOf" +} + // AppSecLoginEvents handles the renaming of appsec login event functions. // TrackUserLoginSuccessEvent → TrackUserLoginSuccess // TrackUserLoginFailureEvent → TrackUserLoginFailure diff --git a/tools/v2fix/v2fix/v2fix_test.go b/tools/v2fix/v2fix/v2fix_test.go index 640bf46279..deb1b0bd2e 100644 --- a/tools/v2fix/v2fix/v2fix_test.go +++ b/tools/v2fix/v2fix/v2fix_test.go @@ -100,6 +100,11 @@ func TestDeprecatedSamplingRules(t *testing.T) { c.Run(testRunner(t, "samplingrules")) } +func TestChildOfStartChild(t *testing.T) { + c := NewChecker(&ChildOfStartChild{}) + c.Run(testRunner(t, "childof")) +} + func TestAppSecLoginEvents(t *testing.T) { c := NewChecker(&AppSecLoginEvents{}) c.Run(testRunner(t, "appseclogin")) From d28def96ad82560bcc14b596c6c076ece42bbf48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:50:15 +0100 Subject: [PATCH 11/30] test(v2fix): extend DDTrace type fixture coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added composite-type cases plus the N constant to the ddtracetypes stage fixtures and taught DDTraceTypes to preserve pointer/slice/array prefixes, skip fixes when array lengths can’t be rendered, and exclude SpanContext even when wrapped so the migration checker can diagnose the expanded scenarios without corrupting the source representation. --- .../v2fix/_stage/ddtracetypes/ddtracetypes.go | 21 ++++++++++++++++++ .../ddtracetypes/ddtracetypes.go.golden | 21 ++++++++++++++++++ tools/v2fix/v2fix/known_change.go | 22 ++++++++++++++----- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go b/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go index 950531cb2f..3e3d84adb1 100644 --- a/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go +++ b/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go @@ -11,6 +11,8 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" ) +const N = 2 + func main() { var ( _ ddtrace.FinishConfig // want `the declared type is in the ddtrace/tracer package now` @@ -23,8 +25,27 @@ func main() { _ ddtrace.StartSpanOption // want `the declared type is in the ddtrace/tracer package now` _ ddtrace.Tracer // want `the declared type is in the ddtrace/tracer package now` _ time.Time + + // Composite type tests: pointer, slice, array + _ *ddtrace.Span // want `the declared type is in the ddtrace/tracer package now` + _ []ddtrace.Span // want `the declared type is in the ddtrace/tracer package now` + _ [3]ddtrace.SpanLink // want `the declared type is in the ddtrace/tracer package now` + + // Non-literal array length: diagnostic emitted but no fix (preserves original formatting) + _ [N + 1]ddtrace.Span // want `the declared type is in the ddtrace/tracer package now` + + // Composite SpanContext types should NOT be migrated (exclusion applies to unwrapped base type) + _ *ddtrace.SpanContext + _ []ddtrace.SpanContext + _ [2]ddtrace.SpanContext ) } func spanConsumer(_ ddtrace.Span) { // want `the declared type is in the ddtrace/tracer package now` } + +func pointerConsumer(_ *ddtrace.Span) { // want `the declared type is in the ddtrace/tracer package now` +} + +func sliceConsumer(_ []ddtrace.Span) { // want `the declared type is in the ddtrace/tracer package now` +} diff --git a/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go.golden b/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go.golden index 4f7b605ce2..f5b7fbc7ae 100644 --- a/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go.golden +++ b/tools/v2fix/_stage/ddtracetypes/ddtracetypes.go.golden @@ -12,6 +12,8 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" ) +const N = 2 + func main() { var ( _ tracer.FinishConfig // want `the declared type is in the ddtrace/tracer package now` @@ -24,8 +26,27 @@ func main() { _ tracer.StartSpanOption // want `the declared type is in the ddtrace/tracer package now` _ tracer.Tracer // want `the declared type is in the ddtrace/tracer package now` _ time.Time + + // Composite type tests: pointer, slice, array + _ *tracer.Span // want `the declared type is in the ddtrace/tracer package now` + _ []tracer.Span // want `the declared type is in the ddtrace/tracer package now` + _ [3]tracer.SpanLink // want `the declared type is in the ddtrace/tracer package now` + + // Non-literal array length: diagnostic emitted but no fix (preserves original formatting) + _ [N + 1]ddtrace.Span // want `the declared type is in the ddtrace/tracer package now` + + // Composite SpanContext types should NOT be migrated (exclusion applies to unwrapped base type) + _ *ddtrace.SpanContext + _ []ddtrace.SpanContext + _ [2]ddtrace.SpanContext ) } func spanConsumer(_ tracer.Span) { // want `the declared type is in the ddtrace/tracer package now` } + +func pointerConsumer(_ *tracer.Span) { // want `the declared type is in the ddtrace/tracer package now` +} + +func sliceConsumer(_ []tracer.Span) { // want `the declared type is in the ddtrace/tracer package now` +} diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 289849cfd2..b7e4c9b0fc 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -180,17 +180,26 @@ func (DDTraceTypes) Clone() KnownChange { } func (c DDTraceTypes) Fixes() []analysis.SuggestedFix { - typ, ok := c.ctx.Value(declaredTypeKey).(*types.Named) - if !ok { + // Skip fix if array length couldn't be rendered (avoid corrupting types) + if skip, _ := c.ctx.Value(skipFixKey).(bool); skip { return nil } - pkg, ok := c.ctx.Value(pkgPrefixKey).(string) - if !ok { + // Get the type name from declaredTypeKey, handling both *types.Named and *types.Alias + // Guard against nil or wrong type to avoid panic on ill-typed code + typ, ok := c.ctx.Value(declaredTypeKey).(types.Type) + if !ok || typ == nil { return nil } + typeObj := getTypeNameFromType(typ) + if typeObj == nil { + return nil + } + + // Get the type prefix (*, [], [N]) if the original type was a composite type + typePrefix, _ := c.ctx.Value(typePrefixKey).(string) - newText := fmt.Sprintf("%s.%s", pkg, typ.Obj().Name()) + newText := fmt.Sprintf("%s%s.%s", typePrefix, c.pkgPrefix(), typeObj.Name()) return []analysis.SuggestedFix{ { Message: "the declared type is in the ddtrace/tracer package now", @@ -214,7 +223,8 @@ func (DDTraceTypes) Probes() []Probe { Is[*ast.Field], ), ImportedFrom("gopkg.in/DataDog/dd-trace-go.v1"), - Not(DeclaresType[ddtrace.SpanContext]()), + // Use HasBaseType to also exclude composite types like *SpanContext, []SpanContext + Not(HasBaseType[ddtrace.SpanContext]()), } } From 56c1318ef180d65858872adc9a1403b67fe2b8d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:51:53 +0100 Subject: [PATCH 12/30] feat(v2fix): cover RateRule in sampling fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a RateRule exercise to the samplingrules fixture and refreshed its golden output to show the expected tracer.Rule{…} literal, while updating DeprecatedSamplingRules to require the v1 package path, keep argument-length guards, and qualify the emitted Rule literal with the package prefix so the fixer now safely rewrites every deprecated constructor (including RateRule) into the new struct form without touching non-dd-trace-go symbols. --- .../_stage/samplingrules/samplingrules.go | 1 + .../samplingrules/samplingrules.go.golden | 27 ++++++++-------- tools/v2fix/v2fix/known_change.go | 32 +++++++++++++++---- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/tools/v2fix/_stage/samplingrules/samplingrules.go b/tools/v2fix/_stage/samplingrules/samplingrules.go index 9a84928632..8204ee4c9b 100644 --- a/tools/v2fix/_stage/samplingrules/samplingrules.go +++ b/tools/v2fix/_stage/samplingrules/samplingrules.go @@ -16,6 +16,7 @@ func tags() map[string]string { func main() { _ = tracer.ServiceRule("test-service", 1.0) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` _ = tracer.NameRule("http.request", 1.0) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.RateRule(0.5) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` _ = tracer.NameServiceRule("http.request", "test-service", 1.0) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` _ = tracer.NameServiceRule("http.*", "test-*", 1.0) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` _ = tracer.ServiceRule("other-service-1", 0.0) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` diff --git a/tools/v2fix/_stage/samplingrules/samplingrules.go.golden b/tools/v2fix/_stage/samplingrules/samplingrules.go.golden index c7a58311e3..0114d668d8 100644 --- a/tools/v2fix/_stage/samplingrules/samplingrules.go.golden +++ b/tools/v2fix/_stage/samplingrules/samplingrules.go.golden @@ -15,17 +15,18 @@ func tags() map[string]string { } func main() { - _ = tracer.TraceSamplingRules(Rule{ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{NameGlob: "http.request", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{NameGlob: "http.request", ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{NameGlob: "http.*", ServiceGlob: "test-*", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{ServiceGlob: "other-service-1", Rate: 0.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{ServiceGlob: "other-service-2", Rate: 0.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{Tags: tags(), ResourceGlob: "", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{Tags: map[string]string{"hostname": "hn-3*"}, ResourceGlob: "res-1*", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.TraceSamplingRules(Rule{Tags: map[string]string{"hostname": "hn-*"}, ResourceGlob: "", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.SpanSamplingRules(Rule{NameGlob: "http.request", ServiceGlob: "test-service", Rate: 1.0, MaxPerSecond: 2.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.SpanSamplingRules(Rule{Tags: tags(), ResourceGlob: "", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` - _ = tracer.SpanSamplingRules(Rule{NameGlob: "http.request", ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{NameGlob: "http.request", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{Rate: 0.5}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{NameGlob: "http.request", ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{NameGlob: "http.*", ServiceGlob: "test-*", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{ServiceGlob: "other-service-1", Rate: 0.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{ServiceGlob: "other-service-2", Rate: 0.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{Tags: tags(), ResourceGlob: "", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{Tags: map[string]string{"hostname": "hn-3*"}, ResourceGlob: "res-1*", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.TraceSamplingRules(tracer.Rule{Tags: map[string]string{"hostname": "hn-*"}, ResourceGlob: "", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.SpanSamplingRules(tracer.Rule{NameGlob: "http.request", ServiceGlob: "test-service", Rate: 1.0, MaxPerSecond: 2.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.SpanSamplingRules(tracer.Rule{Tags: tags(), ResourceGlob: "", NameGlob: "", ServiceGlob: "", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` + _ = tracer.SpanSamplingRules(tracer.Rule{NameGlob: "http.request", ServiceGlob: "test-service", Rate: 1.0}) // want `a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal` } diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index b7e4c9b0fc..de06224ad8 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -422,10 +422,12 @@ func (DeprecatedSamplingRules) Clone() KnownChange { func (c DeprecatedSamplingRules) Probes() []Probe { return []Probe{ IsFuncCall, + HasV1PackagePath, Or( WithFunctionName("ServiceRule"), // Sets funcNameKey WithFunctionName("NameRule"), WithFunctionName("NameServiceRule"), + WithFunctionName("RateRule"), WithFunctionName("TagsResourceRule"), WithFunctionName("SpanNameServiceRule"), WithFunctionName("SpanNameServiceMPSRule"), @@ -445,25 +447,36 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { return nil } - pkg, ok := c.ctx.Value(pkgPrefixKey).(string) - if !ok { - return nil - } - + pkg := c.pkgPrefix() var parts []string switch fn.Name() { case "ServiceRule": + if len(args) < 2 { + return nil + } service := args[0] rate := args[1] parts = append(parts, fmt.Sprintf("ServiceGlob: %s", exprString(service))) parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) case "NameRule": + if len(args) < 2 { + return nil + } name := args[0] rate := args[1] parts = append(parts, fmt.Sprintf("NameGlob: %s", exprString(name))) parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) + case "RateRule": + if len(args) < 1 { + return nil + } + rate := args[0] + parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) case "NameServiceRule", "SpanNameServiceRule": + if len(args) < 3 { + return nil + } name := args[0] service := args[1] rate := args[2] @@ -471,6 +484,9 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { parts = append(parts, fmt.Sprintf("ServiceGlob: %s", exprString(service))) parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) case "SpanNameServiceMPSRule": + if len(args) < 4 { + return nil + } name := args[0] service := args[1] rate := args[2] @@ -480,6 +496,9 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) parts = append(parts, fmt.Sprintf("MaxPerSecond: %s", exprString(limit))) case "TagsResourceRule", "SpanTagsResourceRule": + if len(args) < 5 { + return nil + } tags := args[0] resource := args[1] name := args[2] @@ -500,7 +519,8 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { ruleType = "Trace" } - newText := fmt.Sprintf("%s.%sSamplingRules(Rule{%s})", pkg, ruleType, strings.Join(parts, ", ")) + // Qualify Rule with the package prefix to avoid compilation errors + newText := fmt.Sprintf("%s.%sSamplingRules(%s.Rule{%s})", pkg, ruleType, pkg, strings.Join(parts, ", ")) return []analysis.SuggestedFix{ { From b81f38e2ff7cad88ba0f21750f3e31482097c994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 18:54:59 +0100 Subject: [PATCH 13/30] fix(v2fix): preserve original type qualifier and harden suggested fix generation Use the stored type expression string to keep the original qualifier/alias in struct pointer fixes, fall back to derived types when missing, guard against non-selector calls, and reuse the package prefix helper while tightening argument checks so generated edits are safer and more accurate. --- tools/v2fix/v2fix/known_change.go | 45 +++++++++++++++++-------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index de06224ad8..0deeefa810 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -241,11 +241,17 @@ func (TracerStructs) Clone() KnownChange { } func (c TracerStructs) Fixes() []analysis.SuggestedFix { - typ, ok := c.ctx.Value(declaredTypeKey).(*types.Named) - if !ok { - return nil + // Use the stored type expression string to preserve original qualifier/alias (e.g., "tracer.Span" vs "tr.Span") + typeExprStr, ok := c.ctx.Value(typeExprStrKey).(string) + if !ok || typeExprStr == "" { + // Fallback to building from declared type (handles both *types.Named and *types.Alias) + typ := c.ctx.Value(declaredTypeKey) + typeObj := getTypeNameFromType(typ.(types.Type)) + if typeObj == nil { + return nil + } + typeExprStr = fmt.Sprintf("%s.%s", typeObj.Pkg().Name(), typeObj.Name()) } - typeDecl := fmt.Sprintf("%s.%s", typ.Obj().Pkg().Name(), typ.Obj().Name()) return []analysis.SuggestedFix{ { Message: "the declared type is now a struct, you need to use a pointer", @@ -253,7 +259,7 @@ func (c TracerStructs) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("*%s", typeDecl)), + NewText: []byte(fmt.Sprintf("*%s", typeExprStr)), }, }, }, @@ -287,12 +293,7 @@ func (WithServiceName) Clone() KnownChange { func (c WithServiceName) Fixes() []analysis.SuggestedFix { args, ok := c.ctx.Value(argsKey).([]ast.Expr) - if !ok || args == nil { - return nil - } - - pkg, ok := c.ctx.Value(pkgPrefixKey).(string) - if !ok { + if !ok || len(args) < 1 { return nil } @@ -303,7 +304,7 @@ func (c WithServiceName) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.WithService(%s)", pkg, exprString(args[0]))), + NewText: []byte(fmt.Sprintf("%s.WithService(%s)", c.pkgPrefix(), exprString(args[0]))), }, }, }, @@ -313,6 +314,7 @@ func (c WithServiceName) Fixes() []analysis.SuggestedFix { func (c WithServiceName) Probes() []Probe { return []Probe{ IsFuncCall, + HasV1PackagePath, WithFunctionName("WithServiceName"), } } @@ -340,6 +342,12 @@ func (c TraceIDString) Fixes() []analysis.SuggestedFix { return nil } + // Guard against non-selector callExpr.Fun (e.g., direct function calls) + sel, ok := callExpr.Fun.(*ast.SelectorExpr) + if !ok { + return nil + } + return []analysis.SuggestedFix{ { Message: "trace IDs are now represented as strings, please use TraceIDLower to keep using 64-bits IDs, although it's recommended to switch to 128-bits with TraceID", @@ -347,7 +355,7 @@ func (c TraceIDString) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.TraceIDLower()", exprString(callExpr.Fun.(*ast.SelectorExpr).X))), + NewText: []byte(fmt.Sprintf("%s.TraceIDLower()", exprString(sel.X))), }, }, }, @@ -357,6 +365,7 @@ func (c TraceIDString) Fixes() []analysis.SuggestedFix { func (c TraceIDString) Probes() []Probe { return []Probe{ IsFuncCall, + HasV1PackagePath, WithFunctionName("TraceID"), } } @@ -375,12 +384,7 @@ func (WithDogstatsdAddr) Clone() KnownChange { func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { args, ok := c.ctx.Value(argsKey).([]ast.Expr) - if !ok || args == nil { - return nil - } - - pkg, ok := c.ctx.Value(pkgPrefixKey).(string) - if !ok { + if !ok || len(args) < 1 { return nil } @@ -391,7 +395,7 @@ func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.WithDogstatsdAddr(%s)", pkg, exprString(args[0]))), + NewText: []byte(fmt.Sprintf("%s.WithDogstatsdAddr(%s)", c.pkgPrefix(), exprString(args[0]))), }, }, }, @@ -401,6 +405,7 @@ func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { func (c WithDogstatsdAddr) Probes() []Probe { return []Probe{ IsFuncCall, + HasV1PackagePath, WithFunctionName("WithDogstatsdAddress"), } } From 7bd0cfc776c20020bfafed872e1a1ed6ef809810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 5 Feb 2026 19:06:34 +0100 Subject: [PATCH 14/30] chore(v2fix): make format && make lint --- tools/v2fix/_stage/go.sum | 19 +++++++++++++++++++ tools/v2fix/v2fix/context.go | 8 ++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/tools/v2fix/_stage/go.sum b/tools/v2fix/_stage/go.sum index 396e3d6f98..2d48a450ce 100644 --- a/tools/v2fix/_stage/go.sum +++ b/tools/v2fix/_stage/go.sum @@ -18,6 +18,10 @@ github.com/DataDog/datadog-agent/pkg/version v0.71.0 h1:jqkKmhFrhHSLpiC3twQFDCXU github.com/DataDog/datadog-agent/pkg/version v0.71.0/go.mod h1:FYj51C1ib86rpr5tlLEep9jitqvljIJ5Uz2rrimGTeY= github.com/DataDog/datadog-go/v5 v5.6.0 h1:2oCLxjF/4htd55piM75baflj/KoE6VYS7alEUqFvRDw= github.com/DataDog/datadog-go/v5 v5.6.0/go.mod h1:K9kcYBlxkcPP8tvvjZZKs/m1edNAUFzBbdpTUKfCsuw= +github.com/DataDog/dd-trace-go/contrib/labstack/echo.v4/v2 v2.3.0-rc.2 h1:xw3hOkYNpkbLR+9ZL9gy7b4UpLAbbI8qbhVoTsZBQvs= +github.com/DataDog/dd-trace-go/contrib/labstack/echo.v4/v2 v2.3.0-rc.2/go.mod h1:Yfrm8yh1Nsnod9EVsSlwx5MBJ9uB6B4gjz6bE8A6Fc8= +github.com/DataDog/dd-trace-go/contrib/net/http/v2 v2.3.0-rc.2 h1:zXRv46eiKvATggiQbpclFeTVh/t/fnMtDGMb+m0q65Y= +github.com/DataDog/dd-trace-go/contrib/net/http/v2 v2.3.0-rc.2/go.mod h1:Djb2nb7QCZ89v8RmmQA7V6ZgTGyJZ6NJSozK4Ybmjvo= github.com/DataDog/go-libddwaf/v4 v4.8.0 h1:m6Bl1lS2RtVN4MtdTYhR5vJ2fWQ3WmNy4FiNBpzrp6w= github.com/DataDog/go-libddwaf/v4 v4.8.0/go.mod h1:/AZqP6zw3qGJK5mLrA0PkfK3UQDk1zCI2fUNCt4xftE= github.com/DataDog/go-runtime-metrics-internal v0.0.4-0.20250721125240-fdf1ef85b633 h1:ZRLR9Lbym748e8RznWzmSoK+OfV+8qW6SdNYA4/IqdA= @@ -86,10 +90,19 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/labstack/echo v3.3.10+incompatible h1:pGRcYk231ExFAyoAjAfD85kQzRJCRI8bbnE7CX5OEgg= +github.com/labstack/echo/v4 v4.11.1 h1:dEpLU2FLg4UVmvCGPuk/APjlH6GDpbEPti61srUUUs4= +github.com/labstack/echo/v4 v4.11.1/go.mod h1:YuYRTSM3CHs2ybfrL8Px48bO6BAnYIN4l8wSTMP6BDQ= +github.com/labstack/gommon v0.4.2 h1:F8qTUNXgG1+6WQmqoUWnz8WiEU60mXVVw0P4ht1WRA0= +github.com/labstack/gommon v0.4.2/go.mod h1:QlUFxVM+SNXhDL/Z7YhocGIBYOiwB0mXm1+1bAPHPyU= github.com/linkdata/deadlock v0.5.5 h1:d6O+rzEqasSfamGDA8u7bjtaq7hOX8Ha4Zn36Wxrkvo= github.com/linkdata/deadlock v0.5.5/go.mod h1:tXb28stzAD3trzEEK0UJWC+rZKuobCoPktPYzebb1u0= github.com/lufia/plan9stats v0.0.0-20250317134145-8bc96cf8fc35 h1:PpXWgLPs+Fqr325bN2FD2ISlRRztXibcX6e8f5FR5Dc= github.com/lufia/plan9stats v0.0.0-20250317134145-8bc96cf8fc35/go.mod h1:autxFIvghDt3jPTLoqZ9OZ7s9qTGNAWmYCjVFWPX/zg= +github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= +github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/minio/simdjson-go v0.4.5 h1:r4IQwjRGmWCQ2VeMc7fGiilu1z5du0gJ/I/FsKwgo5A= github.com/minio/simdjson-go v0.4.5/go.mod h1:eoNz0DcLQRyEDeaPr4Ru6JpjlZPzbA0IodxVJk8lO8E= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -153,6 +166,10 @@ github.com/tklauser/numcpus v0.10.0 h1:18njr6LDBk1zuna922MgdjQuJFjrdppsZG60sHGfj github.com/tklauser/numcpus v0.10.0/go.mod h1:BiTKazU708GQTYF4mB+cmlpT2Is1gLk7XVuEeem8LsQ= github.com/trailofbits/go-mutexasserts v0.0.0-20250514102930-c1f3d2e37561 h1:qqa3P9AtNn6RMe90l/lxd3eJWnIRxjI4eb5Rx8xqCLA= github.com/trailofbits/go-mutexasserts v0.0.0-20250514102930-c1f3d2e37561/go.mod h1:GA3+Mq3kt3tYAfM0WZCu7ofy+GW9PuGysHfhr+6JX7s= +github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= +github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= +github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo= +github.com/valyala/fasttemplate v1.2.2/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= github.com/vmihailenco/msgpack/v4 v4.3.13 h1:A2wsiTbvp63ilDaWmsk2wjx6xZdxQOvpiNlKBGKKXKI= github.com/vmihailenco/msgpack/v4 v4.3.13/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= github.com/vmihailenco/tagparser v0.1.2 h1:gnjoVuB/kljJ5wICEEOpx98oXMWPLj22G67Vbd1qPqc= @@ -260,6 +277,8 @@ golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.30.0 h1:yznKA/E9zq54KzlzBEAWn1NXSQ8DIp/NYMy88xJjl4k= +golang.org/x/text v0.30.0/go.mod h1:yDdHFIX9t+tORqspjENWgzaCVXgk0yYnYuSZ8UzzBVM= golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/tools/v2fix/v2fix/context.go b/tools/v2fix/v2fix/context.go index 382021b0f2..a1ffcc76e2 100644 --- a/tools/v2fix/v2fix/context.go +++ b/tools/v2fix/v2fix/context.go @@ -19,8 +19,8 @@ const ( typeKey contextKey = "type" callExprKey contextKey = "call_expr" typeExprStrKey contextKey = "type_expr_str" - typePrefixKey contextKey = "type_prefix" // Stores modifiers like "*", "[]", "[N]" for composite types - skipFixKey contextKey = "skip_fix" // Set to true when a fix cannot be safely applied - childOfParentKey contextKey = "childof_parent" // Stores the parent expression string for ChildOf transformation - childOfOtherOptsKey contextKey = "childof_other" // Stores other options (excluding ChildOf) for StartSpan + typePrefixKey contextKey = "type_prefix" // Stores modifiers like "*", "[]", "[N]" for composite types + skipFixKey contextKey = "skip_fix" // Set to true when a fix cannot be safely applied + childOfParentKey contextKey = "childof_parent" // Stores the parent expression string for ChildOf transformation + childOfOtherOptsKey contextKey = "childof_other" // Stores other options (excluding ChildOf) for StartSpan ) From 5203930b141282d39aa479780470da97d4f2e024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 10 Feb 2026 13:28:51 +0100 Subject: [PATCH 15/30] fix(v2fix): update ChildOf usage to StartChild Refactor the v2fix tool to replace instances of tracer.StartSpan with parent.StartChild when using ChildOf, ensuring compliance with the new API pattern. This change enhances the migration process by aligning with the updated tracing methods while maintaining option handling and test coverage. --- tools/v2fix/_stage/childof/childof.go | 5 ++ tools/v2fix/_stage/childof/childof.go.golden | 4 ++ tools/v2fix/v2fix/known_change.go | 30 +++++++-- tools/v2fix/v2fix/probe.go | 68 +++++++++++++++++++- 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/tools/v2fix/_stage/childof/childof.go b/tools/v2fix/_stage/childof/childof.go index d9e6144b50..12ce625f9f 100644 --- a/tools/v2fix/_stage/childof/childof.go +++ b/tools/v2fix/_stage/childof/childof.go @@ -30,6 +30,11 @@ func main() { child3 := tracer.StartSpan("child3", tracer.ChildOf(parent.Context()), extraOpt) // want `use StartChild instead of StartSpan with ChildOf` defer child3.Finish() + // ChildOf with binary expression in option argument (now supported) + child5 := tracer.StartSpan("child5", tracer.ChildOf(parent.Context()), + tracer.ResourceName("a"+"b")) // want `use StartChild instead of StartSpan with ChildOf` + defer child5.Finish() + // ChildOf passed via variadic (ellipsis applies to ChildOf itself) - should not be rewritten parentOpts := []tracer.StartSpanOption{ tracer.ChildOf(parent.Context()), diff --git a/tools/v2fix/_stage/childof/childof.go.golden b/tools/v2fix/_stage/childof/childof.go.golden index 70f0f8a4d2..cdfc82dec1 100644 --- a/tools/v2fix/_stage/childof/childof.go.golden +++ b/tools/v2fix/_stage/childof/childof.go.golden @@ -31,6 +31,10 @@ func main() { child3 := parent.StartChild("child3", extraOpt) // want `use StartChild instead of StartSpan with ChildOf` defer child3.Finish() + // ChildOf with binary expression in option argument (now supported) + child5 := parent.StartChild("child5", tracer.ResourceName("a" + "b")) // want `use StartChild instead of StartSpan with ChildOf` + defer child5.Finish() + // ChildOf passed via variadic (ellipsis applies to ChildOf itself) - should not be rewritten parentOpts := []tracer.StartSpanOption{ tracer.ChildOf(parent.Context()), diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 0deeefa810..c804251fe1 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -102,7 +102,7 @@ func (d defaultKnownChange) pkgPrefix() string { if pkg, ok := d.ctx.Value(pkgPrefixKey).(string); ok && pkg != "" { return pkg } - return "tracer" + return "" } func eval(k KnownChange, n ast.Node, pass *analysis.Pass) bool { @@ -199,7 +199,11 @@ func (c DDTraceTypes) Fixes() []analysis.SuggestedFix { // Get the type prefix (*, [], [N]) if the original type was a composite type typePrefix, _ := c.ctx.Value(typePrefixKey).(string) - newText := fmt.Sprintf("%s%s.%s", typePrefix, c.pkgPrefix(), typeObj.Name()) + pkg := c.pkgPrefix() + if pkg == "" { + return nil + } + newText := fmt.Sprintf("%s%s.%s", typePrefix, pkg, typeObj.Name()) return []analysis.SuggestedFix{ { Message: "the declared type is in the ddtrace/tracer package now", @@ -297,6 +301,10 @@ func (c WithServiceName) Fixes() []analysis.SuggestedFix { return nil } + pkg := c.pkgPrefix() + if pkg == "" { + return nil + } return []analysis.SuggestedFix{ { Message: "the function WithServiceName is no longer supported. Use WithService instead", @@ -304,7 +312,7 @@ func (c WithServiceName) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.WithService(%s)", c.pkgPrefix(), exprString(args[0]))), + NewText: []byte(fmt.Sprintf("%s.WithService(%s)", pkg, exprString(args[0]))), }, }, }, @@ -388,6 +396,10 @@ func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { return nil } + pkg := c.pkgPrefix() + if pkg == "" { + return nil + } return []analysis.SuggestedFix{ { Message: "the function WithDogstatsdAddress is no longer supported. Use WithDogstatsdAddr instead", @@ -395,7 +407,7 @@ func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.WithDogstatsdAddr(%s)", c.pkgPrefix(), exprString(args[0]))), + NewText: []byte(fmt.Sprintf("%s.WithDogstatsdAddr(%s)", pkg, exprString(args[0]))), }, }, }, @@ -453,6 +465,9 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { } pkg := c.pkgPrefix() + if pkg == "" { + return nil + } var parts []string switch fn.Name() { @@ -608,6 +623,10 @@ func (c ChildOfStartChild) Probes() []Probe { } func (c ChildOfStartChild) Fixes() []analysis.SuggestedFix { + if skip, _ := c.ctx.Value(skipFixKey).(bool); skip { + return nil + } + args, ok := c.ctx.Value(argsKey).([]ast.Expr) if !ok || len(args) < 2 { return nil @@ -684,6 +703,9 @@ func (c AppSecLoginEvents) Fixes() []analysis.SuggestedFix { } pkg := c.pkgPrefix() + if pkg == "" { + return nil + } var newFuncName string switch fn.Name() { case "TrackUserLoginSuccessEvent": diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index d053ed20af..6109bb3516 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -513,6 +513,7 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con var parentExpr string var otherOpts []string foundChildOf := false + skipFix := false isChildOfCall := func(arg ast.Expr) bool { call, ok := arg.(*ast.CallExpr) @@ -532,6 +533,8 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con if !ok { if opt := exprToString(arg); opt != "" { otherOpts = append(otherOpts, opt) + } else { + skipFix = true } continue } @@ -541,6 +544,8 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con if !ok { if opt := exprToString(arg); opt != "" { otherOpts = append(otherOpts, opt) + } else { + skipFix = true } continue } @@ -566,6 +571,8 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con // This is not ChildOf, collect it as another option if opt := exprToString(arg); opt != "" { otherOpts = append(otherOpts, opt) + } else { + skipFix = true } } } @@ -586,6 +593,9 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con otherOpts[len(otherOpts)-1] = otherOpts[len(otherOpts)-1] + "..." } + if skipFix { + ctx = context.WithValue(ctx, skipFixKey, true) + } ctx = context.WithValue(ctx, childOfParentKey, parentExpr) ctx = context.WithValue(ctx, childOfOtherOptsKey, otherOpts) return ctx, true @@ -598,9 +608,21 @@ func exprToString(expr ast.Expr) string { case *ast.Ident: return e.Name case *ast.SelectorExpr: - return exprToString(e.X) + "." + e.Sel.Name + x := exprToString(e.X) + if x == "" { + return "" + } + return x + "." + e.Sel.Name case *ast.CallExpr: - return exprToString(e.Fun) + "(" + exprListToString(e.Args) + ")" + fun := exprToString(e.Fun) + if fun == "" { + return "" + } + args := exprListToString(e.Args) + if args == "" && len(e.Args) > 0 { + return "" + } + return fun + "(" + args + ")" case *ast.BasicLit: return e.Value case *ast.IndexExpr: @@ -611,6 +633,42 @@ func exprToString(expr ast.Expr) string { return e.Op.String() + exprToString(e.X) case *ast.ParenExpr: return "(" + exprToString(e.X) + ")" + case *ast.BinaryExpr: + left := exprToString(e.X) + right := exprToString(e.Y) + if left == "" || right == "" { + return "" + } + return left + " " + e.Op.String() + " " + right + case *ast.SliceExpr: + x := exprToString(e.X) + if x == "" { + return "" + } + low, high := "", "" + if e.Low != nil { + low = exprToString(e.Low) + } + if e.High != nil { + high = exprToString(e.High) + } + if e.Slice3 && e.Max != nil { + return x + "[" + low + ":" + high + ":" + exprToString(e.Max) + "]" + } + return x + "[" + low + ":" + high + "]" + case *ast.CompositeLit: + typ := "" + if e.Type != nil { + typ = exprToString(e.Type) + if typ == "" { + return "" + } + } + elts := exprListToString(e.Elts) + if elts == "" && len(e.Elts) > 0 { + return "" + } + return typ + "{" + elts + "}" } return "" } @@ -618,7 +676,11 @@ func exprToString(expr ast.Expr) string { func exprListToString(exprs []ast.Expr) string { var parts []string for _, e := range exprs { - parts = append(parts, exprToString(e)) + s := exprToString(e) + if s == "" { + return "" + } + parts = append(parts, s) } return strings.Join(parts, ", ") } From 5aebfa06234c984a14d1a7aaf6fa90301d57b4b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 10 Feb 2026 13:29:33 +0100 Subject: [PATCH 16/30] refactor(v2fix): rename getImportPathFromTypeExpr to importPathFromTypeExpr --- tools/v2fix/v2fix/probe.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index 6109bb3516..e48f97cd17 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -273,7 +273,7 @@ func ImportedFrom(pkgPath string) Probe { // Check the AST type expression's import path first (more reliable for aliases). // Use the unwrapped base type expression for the lookup. if baseTypDecl != nil { - if importPath := getImportPathFromTypeExpr(baseTypDecl, pass, n); importPath != "" { + if importPath := importPathFromTypeExpr(baseTypDecl, pass, n); importPath != "" { if strings.HasPrefix(importPath, pkgPath) { return ctx, true } @@ -354,9 +354,9 @@ func getTypeFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass) types.Type { return nil } -// getImportPathFromTypeExpr extracts the import path from a type expression like "pkg.Type". +// importPathFromTypeExpr extracts the import path from a type expression like "pkg.Type". // It looks up the package identifier in pass.TypesInfo.Uses to find the imported package. -func getImportPathFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass, n ast.Node) string { +func importPathFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass, n ast.Node) string { sel, ok := typDecl.(*ast.SelectorExpr) if !ok { return "" From 6a4d4de52bba6ea29c6a9d3e17445ac5122862bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 10 Feb 2026 14:52:30 +0100 Subject: [PATCH 17/30] refactor(v2fix): rename getTypeFromTypeExpr to typeFromTypeExpr --- tools/v2fix/v2fix/probe.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index e48f97cd17..e3841947b6 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -57,7 +57,7 @@ func DeclaresType[T any]() Probe { varType = obj.Type() } else if typDecl != nil { // For blank identifiers, get type from the type expression. - varType = getTypeFromTypeExpr(typDecl, pass) + varType = typeFromTypeExpr(typDecl, pass) } case "*ast.Field": field := n.(*ast.Field) @@ -69,7 +69,7 @@ func DeclaresType[T any]() Probe { if obj != nil { varType = obj.Type() } else if typDecl != nil { - varType = getTypeFromTypeExpr(typDecl, pass) + varType = typeFromTypeExpr(typDecl, pass) } default: return ctx, false @@ -220,7 +220,7 @@ func ImportedFrom(pkgPath string) Probe { if obj != nil { varType = obj.Type() } else if typDecl != nil { - varType = getTypeFromTypeExpr(typDecl, pass) + varType = typeFromTypeExpr(typDecl, pass) } case "*ast.Field": field := n.(*ast.Field) @@ -232,7 +232,7 @@ func ImportedFrom(pkgPath string) Probe { if obj != nil { varType = obj.Type() } else if typDecl != nil { - varType = getTypeFromTypeExpr(typDecl, pass) + varType = typeFromTypeExpr(typDecl, pass) } default: return ctx, false @@ -259,7 +259,7 @@ func ImportedFrom(pkgPath string) Probe { } ctx = context.WithValue(ctx, typePrefixKey, typePrefix) // Also get the base type from the unwrapped expression - varType = getTypeFromTypeExpr(baseTypDecl, pass) + varType = typeFromTypeExpr(baseTypDecl, pass) } } @@ -325,9 +325,9 @@ func unwrapTypeExpr(typDecl ast.Expr) (ast.Expr, string, bool) { } } -// getTypeFromTypeExpr extracts the type from a type expression. +// typeFromTypeExpr extracts the type from a type expression. // This handles various cases including blank identifiers and type aliases. -func getTypeFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass) types.Type { +func typeFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass) types.Type { // Try TypeOf first (works for value expressions) if t := pass.TypesInfo.TypeOf(typDecl); t != nil { return t From be46aaccf25f167731da558fb9c61966a9813b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 10 Feb 2026 14:53:09 +0100 Subject: [PATCH 18/30] refactor(v2fix): replace sort.SliceStable with slices.SortStableFunc for improved sorting --- tools/v2fix/v2fix/golden_generator.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/v2fix/v2fix/golden_generator.go b/tools/v2fix/v2fix/golden_generator.go index 7e7f004712..a6620f0ee8 100644 --- a/tools/v2fix/v2fix/golden_generator.go +++ b/tools/v2fix/v2fix/golden_generator.go @@ -11,6 +11,7 @@ import ( "go/format" "os" "path/filepath" + "slices" "sort" "testing" @@ -34,18 +35,16 @@ func applyEdits(src []byte, edits []diffEdit) ([]byte, error) { } // Sort edits by start position - sorted := make([]diffEdit, len(edits)) - copy(sorted, edits) - sort.SliceStable(sorted, func(i, j int) bool { - if sorted[i].Start != sorted[j].Start { - return sorted[i].Start < sorted[j].Start + slices.SortStableFunc(edits, func(a, b diffEdit) int { + if a.Start != b.Start { + return int(a.Start - b.Start) } - return sorted[i].End < sorted[j].End + return int(a.End - b.End) }) var out bytes.Buffer pos := 0 - for _, edit := range sorted { + for _, edit := range edits { if edit.Start < pos { return nil, fmt.Errorf("overlapping edit at position %d (current pos: %d)", edit.Start, pos) } From e14472fdc857869133fda7a1455afbeb71eea227 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 10 Feb 2026 14:55:48 +0100 Subject: [PATCH 19/30] refactor(v2fix): rename getTypeNameFromType to typeNameFromType This change standardizes the naming convention for type extraction functions in the v2fix tool, improving code clarity and consistency. --- tools/v2fix/v2fix/known_change.go | 4 ++-- tools/v2fix/v2fix/probe.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index c804251fe1..b067622362 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -191,7 +191,7 @@ func (c DDTraceTypes) Fixes() []analysis.SuggestedFix { if !ok || typ == nil { return nil } - typeObj := getTypeNameFromType(typ) + typeObj := typeNameFromType(typ) if typeObj == nil { return nil } @@ -250,7 +250,7 @@ func (c TracerStructs) Fixes() []analysis.SuggestedFix { if !ok || typeExprStr == "" { // Fallback to building from declared type (handles both *types.Named and *types.Alias) typ := c.ctx.Value(declaredTypeKey) - typeObj := getTypeNameFromType(typ.(types.Type)) + typeObj := typeNameFromType(typ.(types.Type)) if typeObj == nil { return nil } diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index e3841947b6..f989b019cd 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -21,8 +21,8 @@ import ( type Probe func(context.Context, ast.Node, *analysis.Pass) (context.Context, bool) -// getTypeNameFromType extracts the TypeName object from Named or Alias types. -func getTypeNameFromType(t types.Type) *types.TypeName { +// typeNameFromType extracts the TypeName object from Named or Alias types. +func typeNameFromType(t types.Type) *types.TypeName { switch t := t.(type) { case *types.Named: return t.Obj() @@ -82,7 +82,7 @@ func DeclaresType[T any]() Probe { } // Extract type object info, handling both *types.Named and *types.Alias - typeObj := getTypeNameFromType(varType) + typeObj := typeNameFromType(varType) if typeObj == nil { return ctx, false } @@ -265,7 +265,7 @@ func ImportedFrom(pkgPath string) Probe { // Store the resolved type in context for use by later probes // Support both *types.Named and *types.Alias (Go 1.22+) - if varType != nil && getTypeNameFromType(varType) != nil { + if varType != nil && typeNameFromType(varType) != nil { ctx = context.WithValue(ctx, declaredTypeKey, varType) } @@ -424,7 +424,7 @@ func HasBaseType[T any]() Probe { if !ok { return ctx, false } - typeObj := getTypeNameFromType(varType) + typeObj := typeNameFromType(varType) if typeObj == nil { return ctx, false } From 1be8406e464f04f47658b181f292f51b04644d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 10 Feb 2026 15:04:39 +0100 Subject: [PATCH 20/30] refactor(v2fix): add docs to context keys --- tools/v2fix/v2fix/context.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tools/v2fix/v2fix/context.go b/tools/v2fix/v2fix/context.go index a1ffcc76e2..d8fc1d05b2 100644 --- a/tools/v2fix/v2fix/context.go +++ b/tools/v2fix/v2fix/context.go @@ -8,17 +8,17 @@ package v2fix type contextKey string const ( - argsKey contextKey = "args" - declaredTypeKey contextKey = "declared_type" - endKey contextKey = "end" - fnKey contextKey = "fn" - pkgNameKey contextKey = "pkg_name" - pkgPrefixKey contextKey = "pkg_prefix" - pkgPathKey contextKey = "pkg_path" - posKey contextKey = "pos" - typeKey contextKey = "type" - callExprKey contextKey = "call_expr" - typeExprStrKey contextKey = "type_expr_str" + argsKey contextKey = "args" // Stores function call arguments as []ast.Expr + declaredTypeKey contextKey = "declared_type" // Stores the declared type as types.Type after type checking + endKey contextKey = "end" // Stores the end position as token.Pos of the AST node + fnKey contextKey = "fn" // Stores the function object as *types.Func being called + pkgNameKey contextKey = "pkg_name" // Stores the package name as string + pkgPrefixKey contextKey = "pkg_prefix" // Stores the package prefix/alias used in selector expressions (e.g., "tracer", "tr") + pkgPathKey contextKey = "pkg_path" // Stores the full package import path as string + posKey contextKey = "pos" // Stores the starting position as token.Pos of the AST node + typeKey contextKey = "type" // Stores the reflect type string representation + callExprKey contextKey = "call_expr" // Stores the call expression AST node as *ast.CallExpr + typeExprStrKey contextKey = "type_expr_str" // Stores the formatted type expression string to preserve original qualifier/alias typePrefixKey contextKey = "type_prefix" // Stores modifiers like "*", "[]", "[N]" for composite types skipFixKey contextKey = "skip_fix" // Set to true when a fix cannot be safely applied childOfParentKey contextKey = "childof_parent" // Stores the parent expression string for ChildOf transformation From 01fc68d3e43f7a30e0618366c35ec2de8b653024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 11 Feb 2026 18:47:37 +0100 Subject: [PATCH 21/30] =?UTF-8?q?fix(v2fix):=20restrict=20ChildOf=E2=86=92?= =?UTF-8?q?StartChild=20autofix=20to=20literal=20operation=20names?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip autofix and emit diagnostic-only warnings when the operation name argument to StartSpan is a non-literal expression (e.g., binary expressions like "a"+suffix). This prevents unsafe rewrites where the operation name cannot be reliably converted to string representation at analysis time. The ChildOf→StartChild transformation now validates that the first argument is a basic literal before applying the suggested fix. Test cases have been added to verify that complex expressions still produce diagnostics but skip the automatic rewrite. --- tools/v2fix/_stage/childof/childof.go | 13 +- tools/v2fix/_stage/childof/childof.go.golden | 10 ++ tools/v2fix/v2fix/known_change.go | 17 ++- tools/v2fix/v2fix/probe.go | 120 +++++++++++++------ 4 files changed, 120 insertions(+), 40 deletions(-) diff --git a/tools/v2fix/_stage/childof/childof.go b/tools/v2fix/_stage/childof/childof.go index 12ce625f9f..ddfe4dcd31 100644 --- a/tools/v2fix/_stage/childof/childof.go +++ b/tools/v2fix/_stage/childof/childof.go @@ -31,10 +31,19 @@ func main() { defer child3.Finish() // ChildOf with binary expression in option argument (now supported) - child5 := tracer.StartSpan("child5", tracer.ChildOf(parent.Context()), - tracer.ResourceName("a"+"b")) // want `use StartChild instead of StartSpan with ChildOf` + child5 := tracer.StartSpan("child5", tracer.ChildOf(parent.Context()), tracer.ResourceName("a"+"b")) // want `use StartChild instead of StartSpan with ChildOf` defer child5.Finish() + // ChildOf with a SpanContext (not a Span) - diagnostic but no fix + parentCtx := parent.Context() + child6 := tracer.StartSpan("child6", tracer.ChildOf(parentCtx)) // want `use StartChild instead of StartSpan with ChildOf` + defer child6.Finish() + + // StartSpan with binary expression as opName - diagnostic but no fix + suffix := "op" + child7 := tracer.StartSpan("a"+suffix, tracer.ChildOf(parent.Context())) // want `use StartChild instead of StartSpan with ChildOf` + defer child7.Finish() + // ChildOf passed via variadic (ellipsis applies to ChildOf itself) - should not be rewritten parentOpts := []tracer.StartSpanOption{ tracer.ChildOf(parent.Context()), diff --git a/tools/v2fix/_stage/childof/childof.go.golden b/tools/v2fix/_stage/childof/childof.go.golden index cdfc82dec1..221125f4de 100644 --- a/tools/v2fix/_stage/childof/childof.go.golden +++ b/tools/v2fix/_stage/childof/childof.go.golden @@ -35,6 +35,16 @@ func main() { child5 := parent.StartChild("child5", tracer.ResourceName("a" + "b")) // want `use StartChild instead of StartSpan with ChildOf` defer child5.Finish() + // ChildOf with a SpanContext (not a Span) - diagnostic but no fix + parentCtx := parent.Context() + child6 := tracer.StartSpan("child6", tracer.ChildOf(parentCtx)) // want `use StartChild instead of StartSpan with ChildOf` + defer child6.Finish() + + // StartSpan with binary expression as opName - diagnostic but no fix + suffix := "op" + child7 := tracer.StartSpan("a"+suffix, tracer.ChildOf(parent.Context())) // want `use StartChild instead of StartSpan with ChildOf` + defer child7.Finish() + // ChildOf passed via variadic (ellipsis applies to ChildOf itself) - should not be rewritten parentOpts := []tracer.StartSpanOption{ tracer.ChildOf(parent.Context()), diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index b067622362..34d47cb090 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -616,7 +616,7 @@ func (ChildOfStartChild) Clone() KnownChange { func (c ChildOfStartChild) Probes() []Probe { return []Probe{ IsFuncCall, - HasV1PackagePath, + HasPackagePrefix("gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"), WithFunctionName("StartSpan"), HasChildOfOption, } @@ -632,8 +632,17 @@ func (c ChildOfStartChild) Fixes() []analysis.SuggestedFix { return nil } - // First arg is the operation name + // First arg is the operation name — only auto-fix when it is a simple + // literal; non-literal expressions (e.g. "a"+suffix) are left as + // diagnostic-only because the rewrite may not be safe. opName := args[0] + if _, isLit := opName.(*ast.BasicLit); !isLit { + return nil + } + opNameStr := exprToString(opName) + if opNameStr == "" { + return nil + } // Get the parent expression from context (set by HasChildOfOption) parentExpr, ok := c.ctx.Value(childOfParentKey).(string) @@ -646,9 +655,9 @@ func (c ChildOfStartChild) Fixes() []analysis.SuggestedFix { var newText string if len(otherOpts) > 0 { - newText = fmt.Sprintf("%s.StartChild(%s, %s)", parentExpr, exprString(opName), strings.Join(otherOpts, ", ")) + newText = fmt.Sprintf("%s.StartChild(%s, %s)", parentExpr, opNameStr, strings.Join(otherOpts, ", ")) } else { - newText = fmt.Sprintf("%s.StartChild(%s)", parentExpr, exprString(opName)) + newText = fmt.Sprintf("%s.StartChild(%s)", parentExpr, opNameStr) } return []analysis.SuggestedFix{ diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index f989b019cd..444ae191bd 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -32,6 +32,26 @@ func typeNameFromType(t types.Type) *types.TypeName { return nil } +// isV1SpanType returns true if the given type is a v1 Span type that maps to +// tracer.Span in v2 (which exposes StartChild). Only ddtrace.Span and +// ddtrace/tracer.Span qualify; other v1 Span types (e.g. mocktracer.Span) +// do not have StartChild in v2. +func isV1SpanType(t types.Type) bool { + if ptr, ok := t.(*types.Pointer); ok { + t = ptr.Elem() + } + tn := typeNameFromType(t) + if tn == nil || tn.Pkg() == nil { + return false + } + if tn.Name() != "Span" { + return false + } + pkgPath := tn.Pkg().Path() + return pkgPath == "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" || + pkgPath == "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +} + // DeclaresType returns true if the node declares a type of the given generic type. // The type use in the generic signature is stored in the context as "type". // The reflected type is stored in the context as "declared_type". @@ -515,6 +535,14 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con foundChildOf := false skipFix := false + collectOpt := func(arg ast.Expr) { + if opt := exprToString(arg); opt != "" { + otherOpts = append(otherOpts, opt) + } else { + skipFix = true + } + } + isChildOfCall := func(arg ast.Expr) bool { call, ok := arg.(*ast.CallExpr) if !ok { @@ -531,56 +559,57 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con for _, arg := range args[1:] { call, ok := arg.(*ast.CallExpr) if !ok { - if opt := exprToString(arg); opt != "" { - otherOpts = append(otherOpts, opt) - } else { - skipFix = true - } + collectOpt(arg) continue } - // Check if this is a ChildOf call sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok { - if opt := exprToString(arg); opt != "" { - otherOpts = append(otherOpts, opt) - } else { - skipFix = true - } + if !ok || sel.Sel.Name != "ChildOf" { + collectOpt(arg) continue } - if sel.Sel.Name == "ChildOf" { - foundChildOf = true - // Extract the parent expression from ChildOf(parent.Context()) or ChildOf(parentCtx) - if len(call.Args) > 0 { - parentArg := call.Args[0] - // Check if it's a parent.Context() call - we want to use just "parent" - if callExpr, ok := parentArg.(*ast.CallExpr); ok { - if selExpr, ok := callExpr.Fun.(*ast.SelectorExpr); ok { - if selExpr.Sel.Name == "Context" { - parentExpr = exprToString(selExpr.X) - continue + // Verify this is dd-trace-go v1 tracer.ChildOf, not a different package's ChildOf + if callee := typeutil.Callee(pass.TypesInfo, call); callee != nil { + if fn, ok := callee.(*types.Func); ok { + if pkg := fn.Pkg(); pkg == nil || !strings.HasPrefix(pkg.Path(), "gopkg.in/DataDog/dd-trace-go.v1") { + // Not a v1 tracer ChildOf; suppress autofix because a + // non-v1 ChildOf helper may wrap parent selection and + // StartChild would override it with its own parent. + skipFix = true + collectOpt(arg) + continue + } + } + } + foundChildOf = true + // Extract the parent expression from ChildOf(parent.Context()) or ChildOf(parentCtx) + if len(call.Args) > 0 { + parentArg := call.Args[0] + // Check if it's a parent.Context() call - we want to use just "parent" + if callExpr, ok := parentArg.(*ast.CallExpr); ok { + if selExpr, ok := callExpr.Fun.(*ast.SelectorExpr); ok { + if selExpr.Sel.Name == "Context" { + if receiverType := pass.TypesInfo.TypeOf(selExpr.X); receiverType == nil || !isV1SpanType(receiverType) { + skipFix = true } + parentExpr = exprToString(selExpr.X) + continue } } - // Otherwise use the full expression - parentExpr = exprToString(parentArg) - } - } else { - // This is not ChildOf, collect it as another option - if opt := exprToString(arg); opt != "" { - otherOpts = append(otherOpts, opt) - } else { - skipFix = true } + // Otherwise use the full expression (ChildOf with SpanContext, not a Span) + skipFix = true + parentExpr = exprToString(parentArg) } } if !foundChildOf || parentExpr == "" { return ctx, false } - // Preserve ellipsis on the last argument if present. + // When variadic opts are present, the slice contents are unknown at + // static analysis time and may contain ChildOf(...) that would change + // parent selection. Suppress the autofix but keep the diagnostic. if hasEllipsis { lastArg := args[len(args)-1] if isChildOfCall(lastArg) { @@ -591,6 +620,7 @@ func HasChildOfOption(ctx context.Context, n ast.Node, pass *analysis.Pass) (con return ctx, false } otherOpts[len(otherOpts)-1] = otherOpts[len(otherOpts)-1] + "..." + skipFix = true } if skipFix { @@ -622,11 +652,19 @@ func exprToString(expr ast.Expr) string { if args == "" && len(e.Args) > 0 { return "" } + if e.Ellipsis.IsValid() { + return fun + "(" + args + "...)" + } return fun + "(" + args + ")" case *ast.BasicLit: return e.Value case *ast.IndexExpr: - return exprToString(e.X) + "[" + exprToString(e.Index) + "]" + x := exprToString(e.X) + idx := exprToString(e.Index) + if x == "" || idx == "" { + return "" + } + return x + "[" + idx + "]" case *ast.StarExpr: return "*" + exprToString(e.X) case *ast.UnaryExpr: @@ -669,6 +707,20 @@ func exprToString(expr ast.Expr) string { return "" } return typ + "{" + elts + "}" + case *ast.KeyValueExpr: + key := exprToString(e.Key) + val := exprToString(e.Value) + if key == "" || val == "" { + return "" + } + return key + ": " + val + case *ast.MapType: + key := exprToString(e.Key) + val := exprToString(e.Value) + if key == "" || val == "" { + return "" + } + return "map[" + key + "]" + val } return "" } From 20b4826a457ed50bce0b02d69bfb0742d4f0645f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 11 Feb 2026 18:50:44 +0100 Subject: [PATCH 22/30] refactor(v2fix): extract and test import path rewriting logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract V1→V2 import path transformation logic into dedicated rewriteV1ImportPath() and rewriteV1ContribImportPath() functions. Add comprehensive test coverage for various import scenarios including core packages, contrib modules, nested subpackages, and longest-prefix matching for contrib module hierarchies. The original inline import path rewriting logic in the V1ImportURL.Fixes() method was difficult to test and didn't properly handle complex contrib module structures --- tools/v2fix/v2fix/known_change.go | 122 ++++++++++++++++++++++--- tools/v2fix/v2fix/known_change_test.go | 65 +++++++++++++ 2 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 tools/v2fix/v2fix/known_change_test.go diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 34d47cb090..66a2f91bbf 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -19,6 +19,82 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" ) +const ( + v1ImportPrefix = "gopkg.in/DataDog/dd-trace-go.v1" + v1ContribImportPrefix = v1ImportPrefix + "/contrib/" + v2ImportPrefix = "github.com/DataDog/dd-trace-go/v2" + v2ContribImportPrefix = "github.com/DataDog/dd-trace-go/contrib/" +) + +// v2ContribModulePaths contains contrib module roots where /v2 must be inserted. +// It mirrors the contrib module layout in this repository (see contrib/**/go.mod). +// We could use instrumentation.GetPackages() to get the list of packages, but it would be +// more complex to derive the v2 import path from TracedPackage field. +var v2ContribModulePaths = []string{ + "99designs/gqlgen", + "IBM/sarama", + "Shopify/sarama", + "aws/aws-sdk-go", + "aws/aws-sdk-go-v2", + "aws/datadog-lambda-go", + "bradfitz/gomemcache", + "cloud.google.com/go/pubsub.v1", + "cloud.google.com/go/pubsub.v2", + "confluentinc/confluent-kafka-go/kafka", + "confluentinc/confluent-kafka-go/kafka.v2", + "database/sql", + "dimfeld/httptreemux.v5", + "elastic/go-elasticsearch.v6", + "emicklei/go-restful.v3", + "envoyproxy/go-control-plane", + "gin-gonic/gin", + "globalsign/mgo", + "go-chi/chi", + "go-chi/chi.v5", + "go-pg/pg.v10", + "go-redis/redis", + "go-redis/redis.v7", + "go-redis/redis.v8", + "go.mongodb.org/mongo-driver", + "go.mongodb.org/mongo-driver.v2", + "gocql/gocql", + "gofiber/fiber.v2", + "gomodule/redigo", + "google.golang.org/api", + "google.golang.org/api/internal/gen_endpoints", + "google.golang.org/grpc", + "gorilla/mux", + "gorm.io/gorm.v1", + "graph-gophers/graphql-go", + "graphql-go/graphql", + "haproxy/stream-processing-offload", + "hashicorp/consul", + "hashicorp/vault", + "jackc/pgx.v5", + "jmoiron/sqlx", + "julienschmidt/httprouter", + "k8s.io/client-go", + "k8s.io/gateway-api", + "labstack/echo.v4", + "log/slog", + "mark3labs/mcp-go", + "miekg/dns", + "modelcontextprotocol/go-sdk", + "net/http", + "olivere/elastic.v5", + "redis/go-redis.v9", + "redis/rueidis", + "segmentio/kafka-go", + "sirupsen/logrus", + "syndtr/goleveldb", + "tidwall/buntdb", + "twitchtv/twirp", + "uptrace/bun", + "urfave/negroni", + "valkey-io/valkey-go", + "valyala/fasthttp", +} + // KnownChange models code expressions that must be changed to migrate to v2. // It is defined by a set of probes that must be true to report the analyzed expression. // It also contains a message function that returns a string describing the change. @@ -120,6 +196,38 @@ func eval(k KnownChange, n ast.Node, pass *analysis.Pass) bool { return true } +func rewriteV1ImportPath(path string) string { + if contribPath, ok := strings.CutPrefix(path, v1ContribImportPrefix); ok { + return rewriteV1ContribImportPath(contribPath) + } + return strings.Replace(path, v1ImportPrefix, v2ImportPrefix, 1) +} + +func rewriteV1ContribImportPath(contribPath string) string { + modulePath := contribPath + subpkgPath := "" + longestMatch := "" + for _, candidate := range v2ContribModulePaths { + if contribPath != candidate && !strings.HasPrefix(contribPath, candidate+"/") { + continue + } + if len(candidate) > len(longestMatch) { + longestMatch = candidate + } + } + if longestMatch != "" { + modulePath = longestMatch + subpkgPath = strings.TrimPrefix(contribPath, longestMatch) + subpkgPath = strings.TrimPrefix(subpkgPath, "/") + } + + path := v2ContribImportPrefix + modulePath + "/v2" + if subpkgPath != "" { + path += "/" + subpkgPath + } + return path +} + type V1ImportURL struct { defaultKnownChange } @@ -134,17 +242,7 @@ func (c V1ImportURL) Fixes() []analysis.SuggestedFix { return nil } - const v1Prefix = "gopkg.in/DataDog/dd-trace-go.v1" - const contribPrefix = v1Prefix + "/contrib/" - - if strings.HasPrefix(path, contribPrefix) { - // Contrib imports: gopkg.in/DataDog/dd-trace-go.v1/contrib/X → github.com/DataDog/dd-trace-go/contrib/X/v2 - contribPath := strings.TrimPrefix(path, contribPrefix) - path = "github.com/DataDog/dd-trace-go/contrib/" + contribPath + "/v2" - } else { - // Core imports: gopkg.in/DataDog/dd-trace-go.v1/X → github.com/DataDog/dd-trace-go/v2/X - path = strings.Replace(path, v1Prefix, "github.com/DataDog/dd-trace-go/v2", 1) - } + path = rewriteV1ImportPath(path) return []analysis.SuggestedFix{ { @@ -153,7 +251,7 @@ func (c V1ImportURL) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%q", path)), + NewText: fmt.Appendf(nil, "%q", path), }, }, }, diff --git a/tools/v2fix/v2fix/known_change_test.go b/tools/v2fix/v2fix/known_change_test.go new file mode 100644 index 0000000000..9c76713957 --- /dev/null +++ b/tools/v2fix/v2fix/known_change_test.go @@ -0,0 +1,65 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2023 Datadog, Inc. + +package v2fix + +import "testing" + +func TestRewriteV1ImportPath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in string + want string + }{ + { + name: "core package", + in: "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer", + want: "github.com/DataDog/dd-trace-go/v2/ddtrace/tracer", + }, + { + name: "contrib module root", + in: "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http", + want: "github.com/DataDog/dd-trace-go/contrib/net/http/v2", + }, + { + name: "contrib subpackage", + in: "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http/client", + want: "github.com/DataDog/dd-trace-go/contrib/net/http/v2/client", + }, + { + name: "contrib nested module root", + in: "gopkg.in/DataDog/dd-trace-go.v1/contrib/confluentinc/confluent-kafka-go/kafka", + want: "github.com/DataDog/dd-trace-go/contrib/confluentinc/confluent-kafka-go/kafka/v2", + }, + { + name: "contrib nested module subpackage", + in: "gopkg.in/DataDog/dd-trace-go.v1/contrib/confluentinc/confluent-kafka-go/kafka/producer", + want: "github.com/DataDog/dd-trace-go/contrib/confluentinc/confluent-kafka-go/kafka/v2/producer", + }, + { + name: "longest module prefix wins", + in: "gopkg.in/DataDog/dd-trace-go.v1/contrib/google.golang.org/api/internal/gen_endpoints/config", + want: "github.com/DataDog/dd-trace-go/contrib/google.golang.org/api/internal/gen_endpoints/v2/config", + }, + { + name: "unknown contrib fallback", + in: "gopkg.in/DataDog/dd-trace-go.v1/contrib/acme/custom/pkg", + want: "github.com/DataDog/dd-trace-go/contrib/acme/custom/pkg/v2", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := rewriteV1ImportPath(tt.in) + if got != tt.want { + t.Fatalf("rewriteV1ImportPath(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} From 2cbdd6d4f321f90579246a26623780b8454d6206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 11 Feb 2026 18:54:41 +0100 Subject: [PATCH 23/30] refactor(v2fix): deduplicate type extraction and matching logic in probes Extract common type extraction and matching patterns into reusable helpers: - extractTypeInfo() consolidates ValueSpec/Field type extraction logic - matchesGenericType[T]() encapsulates generic type matching via reflection The probe functions DeclaresType, ImportedFrom, and HasBaseType contained significant code duplication for extracting type information from AST nodes and matching types against generic constraints. --- tools/v2fix/v2fix/probe.go | 164 ++++++++++++------------------------- 1 file changed, 51 insertions(+), 113 deletions(-) diff --git a/tools/v2fix/v2fix/probe.go b/tools/v2fix/v2fix/probe.go index 444ae191bd..3ddb986dcd 100644 --- a/tools/v2fix/v2fix/probe.go +++ b/tools/v2fix/v2fix/probe.go @@ -52,6 +52,46 @@ func isV1SpanType(t types.Type) bool { pkgPath == "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" } +// extractTypeInfo extracts the type declaration expression and resolved type from +// a ValueSpec or Field node. It returns the type expression, the resolved type, +// and whether extraction succeeded. This is shared by DeclaresType and ImportedFrom. +func extractTypeInfo(ctx context.Context, n ast.Node, pass *analysis.Pass) (ast.Expr, types.Type, bool) { + var names []*ast.Ident + var typDecl ast.Expr + switch ctx.Value(typeKey) { + case "*ast.ValueSpec": + spec := n.(*ast.ValueSpec) + names = spec.Names + typDecl = spec.Type + case "*ast.Field": + field := n.(*ast.Field) + names = field.Names + typDecl = field.Type + default: + return nil, nil, false + } + if len(names) == 0 { + return nil, nil, false + } + if obj := pass.TypesInfo.ObjectOf(names[0]); obj != nil { + return typDecl, obj.Type(), true + } + if typDecl != nil { + return typDecl, typeFromTypeExpr(typDecl, pass), true + } + return nil, nil, false +} + +// matchesGenericType returns true if typeObj matches the generic type T +// by comparing package path and type name via reflection. +func matchesGenericType[T any](typeObj *types.TypeName) bool { + if typeObj == nil || typeObj.Pkg() == nil { + return false + } + e := reflect.TypeFor[T]() + return typeObj.Pkg().Path() == e.PkgPath() && typeObj.Name() == e.Name() +} + // DeclaresType returns true if the node declares a type of the given generic type. // The type use in the generic signature is stored in the context as "type". // The reflected type is stored in the context as "declared_type". @@ -59,51 +99,13 @@ func isV1SpanType(t types.Type) bool { // Handles both *types.Named and *types.Alias (Go 1.22+ type aliases). func DeclaresType[T any]() Probe { return func(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { - var ( - typ = ctx.Value(typeKey) - typDecl ast.Expr - varType types.Type - ) - switch typ { - case "*ast.ValueSpec": - spec := n.(*ast.ValueSpec) - if len(spec.Names) == 0 { - return ctx, false - } - typDecl = spec.Type - // Try to get type from object first (works for named vars) - obj := pass.TypesInfo.ObjectOf(spec.Names[0]) - if obj != nil { - varType = obj.Type() - } else if typDecl != nil { - // For blank identifiers, get type from the type expression. - varType = typeFromTypeExpr(typDecl, pass) - } - case "*ast.Field": - field := n.(*ast.Field) - if len(field.Names) == 0 { - return ctx, false - } - typDecl = field.Type - obj := pass.TypesInfo.ObjectOf(field.Names[0]) - if obj != nil { - varType = obj.Type() - } else if typDecl != nil { - varType = typeFromTypeExpr(typDecl, pass) - } - default: - return ctx, false - } - if typDecl == nil { - return ctx, false - } - if varType == nil { + typDecl, varType, ok := extractTypeInfo(ctx, n, pass) + if !ok || typDecl == nil || varType == nil { return ctx, false } - // Extract type object info, handling both *types.Named and *types.Alias typeObj := typeNameFromType(varType) - if typeObj == nil { + if !matchesGenericType[T](typeObj) { return ctx, false } ctx = context.WithValue(ctx, declaredTypeKey, varType) @@ -117,18 +119,7 @@ func DeclaresType[T any]() Probe { ctx = context.WithValue(ctx, typeExprStrKey, buf.String()) } - v := new(T) - e := reflect.TypeOf(v).Elem() - if typeObj.Pkg() == nil { - return ctx, false - } ctx = context.WithValue(ctx, pkgNameKey, typeObj.Pkg().Name()) - if typeObj.Pkg().Path() != e.PkgPath() { - return ctx, false - } - if typeObj.Name() != e.Name() { - return ctx, false - } return ctx, true } } @@ -224,37 +215,8 @@ func HasPackagePrefix(prefix string) Probe { // and storing the type prefix (e.g., "*", "[]") in typePrefixKey. func ImportedFrom(pkgPath string) Probe { return func(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { - var ( - typ = ctx.Value(typeKey) - varType types.Type - typDecl ast.Expr - ) - switch typ { - case "*ast.ValueSpec": - spec := n.(*ast.ValueSpec) - if len(spec.Names) == 0 { - return ctx, false - } - typDecl = spec.Type - obj := pass.TypesInfo.ObjectOf(spec.Names[0]) - if obj != nil { - varType = obj.Type() - } else if typDecl != nil { - varType = typeFromTypeExpr(typDecl, pass) - } - case "*ast.Field": - field := n.(*ast.Field) - if len(field.Names) == 0 { - return ctx, false - } - typDecl = field.Type - obj := pass.TypesInfo.ObjectOf(field.Names[0]) - if obj != nil { - varType = obj.Type() - } else if typDecl != nil { - varType = typeFromTypeExpr(typDecl, pass) - } - default: + typDecl, varType, ok := extractTypeInfo(ctx, n, pass) + if !ok { return ctx, false } @@ -404,23 +366,19 @@ func importPathFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass, n ast.Node) s for _, file := range pass.Files { if file.Pos() <= nodePos && nodePos < file.End() { for _, imp := range file.Imports { + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + continue + } name := "" if imp.Name != nil { name = imp.Name.Name } else { // Use the last part of the path as the default name - path, err := strconv.Unquote(imp.Path.Value) - if err != nil { - continue - } parts := strings.Split(path, "/") name = parts[len(parts)-1] } if name == ident.Name { - path, err := strconv.Unquote(imp.Path.Value) - if err != nil { - continue - } return path } } @@ -436,31 +394,11 @@ func importPathFromTypeExpr(typDecl ast.Expr, pass *analysis.Pass, n ast.Node) s // It expects declaredTypeKey to be set by ImportedFrom (which stores the unwrapped type). func HasBaseType[T any]() Probe { return func(ctx context.Context, n ast.Node, pass *analysis.Pass) (context.Context, bool) { - declaredType := ctx.Value(declaredTypeKey) - if declaredType == nil { - return ctx, false - } - varType, ok := declaredType.(types.Type) + varType, ok := ctx.Value(declaredTypeKey).(types.Type) if !ok { return ctx, false } - typeObj := typeNameFromType(varType) - if typeObj == nil { - return ctx, false - } - if typeObj.Pkg() == nil { - return ctx, false - } - - v := new(T) - e := reflect.TypeOf(v).Elem() - if typeObj.Pkg().Path() != e.PkgPath() { - return ctx, false - } - if typeObj.Name() != e.Name() { - return ctx, false - } - return ctx, true + return ctx, matchesGenericType[T](typeNameFromType(varType)) } } From ee88b1f1770c9d2bb14826c6e30529a253f494b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:16:30 +0100 Subject: [PATCH 24/30] fix(v2fix): disable autofix for appsec login event function renames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove automatic rewrite logic from AppSecLoginEvents analyzer and make it diagnostic-only. The v1→v2 migration for these functions involves incompatible parameter signature changes that cannot be safely automated: - TrackUserLoginSuccessEvent gained a new 'login' parameter - TrackUserLoginFailureEvent's second parameter semantics changed from user ID to login value --- .../_stage/appseclogin/appseclogin.go.golden | 4 +- tools/v2fix/v2fix/known_change.go | 43 +++---------------- 2 files changed, 8 insertions(+), 39 deletions(-) diff --git a/tools/v2fix/_stage/appseclogin/appseclogin.go.golden b/tools/v2fix/_stage/appseclogin/appseclogin.go.golden index 2fca9a61d6..e8eb2bffc7 100644 --- a/tools/v2fix/_stage/appseclogin/appseclogin.go.golden +++ b/tools/v2fix/_stage/appseclogin/appseclogin.go.golden @@ -16,8 +16,8 @@ func main() { ctx := context.Background() // Login success event - appsec.TrackUserLoginSuccess(ctx, "user123", nil) // want `appsec login event functions have been renamed` + appsec.TrackUserLoginSuccessEvent(ctx, "user123", nil) // want `appsec login event functions have been renamed` // Login failure event - appsec.TrackUserLoginFailure(ctx, "user123", false, nil) // want `appsec login event functions have been renamed` + appsec.TrackUserLoginFailureEvent(ctx, "user123", false, nil) // want `appsec login event functions have been renamed` } diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 66a2f91bbf..ed548829cd 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -799,43 +799,12 @@ func (c AppSecLoginEvents) Probes() []Probe { } func (c AppSecLoginEvents) Fixes() []analysis.SuggestedFix { - fn, ok := c.ctx.Value(fnKey).(*types.Func) - if !ok || fn == nil { - return nil - } - - args, ok := c.ctx.Value(argsKey).([]ast.Expr) - if !ok { - return nil - } - - pkg := c.pkgPrefix() - if pkg == "" { - return nil - } - var newFuncName string - switch fn.Name() { - case "TrackUserLoginSuccessEvent": - newFuncName = "TrackUserLoginSuccess" - case "TrackUserLoginFailureEvent": - newFuncName = "TrackUserLoginFailure" - default: - return nil - } - - newText := fmt.Sprintf("%s.%s(%s)", pkg, newFuncName, exprListString(args)) - return []analysis.SuggestedFix{ - { - Message: "appsec login event functions have been renamed (remove 'Event' suffix)", - TextEdits: []analysis.TextEdit{ - { - Pos: c.Pos(), - End: c.End(), - NewText: []byte(newText), - }, - }, - }, - } + // Neither function can be safely auto-fixed: + // - TrackUserLoginSuccessEvent → TrackUserLoginSuccess has a new 'login' parameter + // - TrackUserLoginFailureEvent → TrackUserLoginFailure's second parameter changed + // from user ID to login value + // Both are diagnostic-only until a safe argument mapping is available. + return nil } func (c AppSecLoginEvents) String() string { From 586d638a6df75ebe6897e12d4243960be3484353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:20:59 +0100 Subject: [PATCH 25/30] fix(v2fix): avoid unnecessary mutation in contextHandler.Context() Return context.Background() directly when c.ctx is nil instead of attempting to assign it first. Since contextHandler.Context() uses a value receiver, the assignment c.ctx = context.Background() had no effect outside the method scope and was unnecessarily confusing. --- tools/v2fix/v2fix/known_change.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index ed548829cd..52feebac19 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -130,7 +130,7 @@ type contextHandler struct { func (c contextHandler) Context() context.Context { if c.ctx == nil { - c.ctx = context.Background() + return context.Background() } return c.ctx } From 0414d3a17d3f43c4be0f39c9fbc9e63fb41b5b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:26:49 +0100 Subject: [PATCH 26/30] refactor(v2fix): optimize and simplify code generation in known_change.go - Remove unused bytes import - Simplify pkgPrefix() to return value directly without redundant checks - Replace []byte(fmt.Sprintf(...)) with fmt.Appendf(nil, ...) for better performance and reduced allocations - Consistently use exprToString() instead of exprString() where appropriate - Add type assertion validation for declaredTypeKey context value - Remove unnecessary function pointer validation in TraceIDString.Fixes() --- tools/v2fix/v2fix/known_change.go | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 52feebac19..210c160583 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -6,7 +6,6 @@ package v2fix import ( - "bytes" "context" "fmt" "go/ast" @@ -175,10 +174,8 @@ func (d defaultKnownChange) Pos() token.Pos { } func (d defaultKnownChange) pkgPrefix() string { - if pkg, ok := d.ctx.Value(pkgPrefixKey).(string); ok && pkg != "" { - return pkg - } - return "" + pkg, _ := d.ctx.Value(pkgPrefixKey).(string) + return pkg } func eval(k KnownChange, n ast.Node, pass *analysis.Pass) bool { @@ -301,7 +298,6 @@ func (c DDTraceTypes) Fixes() []analysis.SuggestedFix { if pkg == "" { return nil } - newText := fmt.Sprintf("%s%s.%s", typePrefix, pkg, typeObj.Name()) return []analysis.SuggestedFix{ { Message: "the declared type is in the ddtrace/tracer package now", @@ -309,7 +305,7 @@ func (c DDTraceTypes) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(newText), + NewText: fmt.Appendf(nil, "%s%s.%s", typePrefix, pkg, typeObj.Name()), }, }, }, @@ -347,8 +343,11 @@ func (c TracerStructs) Fixes() []analysis.SuggestedFix { typeExprStr, ok := c.ctx.Value(typeExprStrKey).(string) if !ok || typeExprStr == "" { // Fallback to building from declared type (handles both *types.Named and *types.Alias) - typ := c.ctx.Value(declaredTypeKey) - typeObj := typeNameFromType(typ.(types.Type)) + typ, ok := c.ctx.Value(declaredTypeKey).(types.Type) + if !ok { + return nil + } + typeObj := typeNameFromType(typ) if typeObj == nil { return nil } @@ -361,7 +360,7 @@ func (c TracerStructs) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("*%s", typeExprStr)), + NewText: fmt.Appendf(nil, "*%s", typeExprStr), }, }, }, @@ -410,7 +409,7 @@ func (c WithServiceName) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.WithService(%s)", pkg, exprString(args[0]))), + NewText: fmt.Appendf(nil, "%s.WithService(%s)", pkg, exprToString(args[0])), }, }, }, @@ -438,11 +437,6 @@ func (TraceIDString) Clone() KnownChange { } func (c TraceIDString) Fixes() []analysis.SuggestedFix { - fn, ok := c.ctx.Value(fnKey).(*types.Func) - if !ok || fn == nil { - return nil - } - callExpr, ok := c.ctx.Value(callExprKey).(*ast.CallExpr) if !ok { return nil @@ -461,7 +455,7 @@ func (c TraceIDString) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.TraceIDLower()", exprString(sel.X))), + NewText: fmt.Appendf(nil, "%s.TraceIDLower()", exprToString(sel.X)), }, }, }, @@ -505,7 +499,7 @@ func (c WithDogstatsdAddr) Fixes() []analysis.SuggestedFix { { Pos: c.Pos(), End: c.End(), - NewText: []byte(fmt.Sprintf("%s.WithDogstatsdAddr(%s)", pkg, exprString(args[0]))), + NewText: fmt.Appendf(nil, "%s.WithDogstatsdAddr(%s)", pkg, exprToString(args[0])), }, }, }, From 543f1ff67177b1c8d13142fa143838aed41a5d7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:28:46 +0100 Subject: [PATCH 27/30] refactor(v2fix): consolidate on exprToString and optimize SamplingRule fixes Remove duplicate exprString() helper functions (exprString, exprListString, exprCompositeString) from known_change.go in favor of using exprToString() from probe.go. Optimize SamplingRuleConstructor.Fixes() by pre-rendering all arguments once and bailing early if any expression cannot be safely converted to string representation. --- tools/v2fix/v2fix/known_change.go | 106 ++++++++---------------------- 1 file changed, 29 insertions(+), 77 deletions(-) diff --git a/tools/v2fix/v2fix/known_change.go b/tools/v2fix/v2fix/known_change.go index 210c160583..7d9be93203 100644 --- a/tools/v2fix/v2fix/known_change.go +++ b/tools/v2fix/v2fix/known_change.go @@ -560,6 +560,18 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { if pkg == "" { return nil } + + // Pre-render all args to strings once; bail if any can't be rendered + // (e.g. binary expressions like 1.0/2 produce empty values). + argStrs := make([]string, len(args)) + for i, arg := range args { + s := exprToString(arg) + if s == "" { + return nil + } + argStrs[i] = s + } + var parts []string switch fn.Name() { @@ -567,60 +579,43 @@ func (c DeprecatedSamplingRules) Fixes() []analysis.SuggestedFix { if len(args) < 2 { return nil } - service := args[0] - rate := args[1] - parts = append(parts, fmt.Sprintf("ServiceGlob: %s", exprString(service))) - parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) + parts = append(parts, fmt.Sprintf("ServiceGlob: %s", argStrs[0])) + parts = append(parts, fmt.Sprintf("Rate: %s", argStrs[1])) case "NameRule": if len(args) < 2 { return nil } - name := args[0] - rate := args[1] - parts = append(parts, fmt.Sprintf("NameGlob: %s", exprString(name))) - parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) + parts = append(parts, fmt.Sprintf("NameGlob: %s", argStrs[0])) + parts = append(parts, fmt.Sprintf("Rate: %s", argStrs[1])) case "RateRule": if len(args) < 1 { return nil } - rate := args[0] - parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) + parts = append(parts, fmt.Sprintf("Rate: %s", argStrs[0])) case "NameServiceRule", "SpanNameServiceRule": if len(args) < 3 { return nil } - name := args[0] - service := args[1] - rate := args[2] - parts = append(parts, fmt.Sprintf("NameGlob: %s", exprString(name))) - parts = append(parts, fmt.Sprintf("ServiceGlob: %s", exprString(service))) - parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) + parts = append(parts, fmt.Sprintf("NameGlob: %s", argStrs[0])) + parts = append(parts, fmt.Sprintf("ServiceGlob: %s", argStrs[1])) + parts = append(parts, fmt.Sprintf("Rate: %s", argStrs[2])) case "SpanNameServiceMPSRule": if len(args) < 4 { return nil } - name := args[0] - service := args[1] - rate := args[2] - limit := args[3] - parts = append(parts, fmt.Sprintf("NameGlob: %s", exprString(name))) - parts = append(parts, fmt.Sprintf("ServiceGlob: %s", exprString(service))) - parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) - parts = append(parts, fmt.Sprintf("MaxPerSecond: %s", exprString(limit))) + parts = append(parts, fmt.Sprintf("NameGlob: %s", argStrs[0])) + parts = append(parts, fmt.Sprintf("ServiceGlob: %s", argStrs[1])) + parts = append(parts, fmt.Sprintf("Rate: %s", argStrs[2])) + parts = append(parts, fmt.Sprintf("MaxPerSecond: %s", argStrs[3])) case "TagsResourceRule", "SpanTagsResourceRule": if len(args) < 5 { return nil } - tags := args[0] - resource := args[1] - name := args[2] - service := args[3] - rate := args[4] - parts = append(parts, fmt.Sprintf("Tags: %s", exprString(tags))) - parts = append(parts, fmt.Sprintf("ResourceGlob: %s", exprString(resource))) - parts = append(parts, fmt.Sprintf("NameGlob: %s", exprString(name))) - parts = append(parts, fmt.Sprintf("ServiceGlob: %s", exprString(service))) - parts = append(parts, fmt.Sprintf("Rate: %s", exprString(rate))) + parts = append(parts, fmt.Sprintf("Tags: %s", argStrs[0])) + parts = append(parts, fmt.Sprintf("ResourceGlob: %s", argStrs[1])) + parts = append(parts, fmt.Sprintf("NameGlob: %s", argStrs[2])) + parts = append(parts, fmt.Sprintf("ServiceGlob: %s", argStrs[3])) + parts = append(parts, fmt.Sprintf("Rate: %s", argStrs[4])) } var ruleType string @@ -652,49 +647,6 @@ func (c DeprecatedSamplingRules) String() string { return "a deprecated sampling rule constructor function should be replaced with a tracer.Rule{...} struct literal" } -func exprListString(exprs []ast.Expr) string { - var buf bytes.Buffer - for i, expr := range exprs { - if i > 0 { - buf.WriteString(", ") - } - buf.WriteString(exprString(expr)) - } - return buf.String() -} - -func exprCompositeString(expr *ast.CompositeLit) string { - var buf bytes.Buffer - buf.WriteString(exprString(expr.Type)) - buf.WriteString("{") - for _, expr := range expr.Elts { - buf.WriteString(exprString(expr)) - buf.WriteString(",") - } - buf.WriteString("}") - return buf.String() -} - -func exprString(expr ast.Expr) string { - switch expr := expr.(type) { - case *ast.SelectorExpr: - return exprString(expr.X) + "." + exprString(expr.Sel) - case *ast.CompositeLit: - return exprCompositeString(expr) - case *ast.KeyValueExpr: - return exprString(expr.Key) + ":" + exprString(expr.Value) - case *ast.MapType: - return "map[" + exprString(expr.Key) + "]" + exprString(expr.Value) - case *ast.BasicLit: - return expr.Value - case *ast.Ident: - return expr.Name - case *ast.CallExpr: - return exprString(expr.Fun) + "(" + exprListString(expr.Args) + ")" - } - return "" -} - // ChildOfStartChild handles the transformation of tracer.StartSpan("op", tracer.ChildOf(parent.Context())) // to parent.StartChild("op"). This is a complex structural change. type ChildOfStartChild struct { From da5f7ccde54e8a3042d4d40599264c12e7bd8b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:29:16 +0100 Subject: [PATCH 28/30] refactor(v2fix): enhance golden_generator for multi-message support Update golden_generator.go to handle multiple messages by creating a txtar archive with multiple sections. This change improves the formatting logic and ensures consistent output for both single and multiple messages, while maintaining the requirement for the golden content to end with a newline. --- tools/v2fix/v2fix/golden_generator.go | 29 ++++----------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/tools/v2fix/v2fix/golden_generator.go b/tools/v2fix/v2fix/golden_generator.go index a6620f0ee8..47da19109d 100644 --- a/tools/v2fix/v2fix/golden_generator.go +++ b/tools/v2fix/v2fix/golden_generator.go @@ -134,20 +134,18 @@ func runWithSuggestedFixesUpdate(t *testing.T, dir string, a *analysis.Analyzer, var golden bytes.Buffer - if len(messages) == 1 { - // Single message: use simple txtar format with one section - msg := messages[0] + multiMessage := len(messages) > 1 + for _, msg := range messages { edits := fixes[msg] out, err := applyEdits(orig, edits) if err != nil { - t.Errorf("error applying edits to %s: %v", fileName, err) + t.Errorf("error applying edits to %s for message %q: %v", fileName, msg, err) continue } formatted, err := format.Source(out) if err != nil { - // If formatting fails, use unformatted formatted = out } @@ -155,26 +153,7 @@ func runWithSuggestedFixesUpdate(t *testing.T, dir string, a *analysis.Analyzer, golden.WriteString(msg) golden.WriteString(" --\n") golden.Write(formatted) - } else { - // Multiple messages: create txtar archive with multiple sections - for _, msg := range messages { - edits := fixes[msg] - - out, err := applyEdits(orig, edits) - if err != nil { - t.Errorf("error applying edits to %s for message %q: %v", fileName, msg, err) - continue - } - - formatted, err := format.Source(out) - if err != nil { - formatted = out - } - - golden.WriteString("-- ") - golden.WriteString(msg) - golden.WriteString(" --\n") - golden.Write(formatted) + if multiMessage { golden.WriteString("\n") } } From 8bdace6b7086ea9f3b0616a5545bde78e151e518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:29:37 +0100 Subject: [PATCH 29/30] refactor(v2fix): improve diagnostic reporting structure Enhance the diagnostic reporting in v2fix by ensuring that the message and suggested fixes are properly structured. This change aims to streamline the reporting process and improve clarity in the output diagnostics. --- tools/v2fix/v2fix/v2fix.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/v2fix/v2fix/v2fix.go b/tools/v2fix/v2fix/v2fix.go index 5d8be7d940..55fa2b6660 100644 --- a/tools/v2fix/v2fix/v2fix.go +++ b/tools/v2fix/v2fix/v2fix.go @@ -64,11 +64,8 @@ func (c Checker) runner() func(*analysis.Pass) (interface{}, error) { pass.Report(analysis.Diagnostic{ Pos: n.Pos(), End: n.End(), - Category: "", Message: k.String(), - URL: "", SuggestedFixes: k.Fixes(), - Related: nil, }) }) return nil, nil From 84322c80d41d8a6ef608cec6729d34d8d9553f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Feb 2026 09:35:53 +0100 Subject: [PATCH 30/30] refactor(v2fix): update comments for clarity on ChildOf usage Revise comments in childof.go.golden to enhance clarity regarding the use of StartChild instead of StartSpan with ChildOf. This change aims to improve understanding of the binary expression handling in option arguments and the diagnostic context for SpanContext usage. --- tools/v2fix/_stage/childof/childof.go.golden | 2 +- tools/v2fix/_stage/v1usage/v1usage.go.golden | 57 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 tools/v2fix/_stage/v1usage/v1usage.go.golden diff --git a/tools/v2fix/_stage/childof/childof.go.golden b/tools/v2fix/_stage/childof/childof.go.golden index 221125f4de..dcf7f5124d 100644 --- a/tools/v2fix/_stage/childof/childof.go.golden +++ b/tools/v2fix/_stage/childof/childof.go.golden @@ -32,7 +32,7 @@ func main() { defer child3.Finish() // ChildOf with binary expression in option argument (now supported) - child5 := parent.StartChild("child5", tracer.ResourceName("a" + "b")) // want `use StartChild instead of StartSpan with ChildOf` + child5 := parent.StartChild("child5", tracer.ResourceName("a"+"b")) // want `use StartChild instead of StartSpan with ChildOf` defer child5.Finish() // ChildOf with a SpanContext (not a Span) - diagnostic but no fix diff --git a/tools/v2fix/_stage/v1usage/v1usage.go.golden b/tools/v2fix/_stage/v1usage/v1usage.go.golden new file mode 100644 index 0000000000..88cbeb9fac --- /dev/null +++ b/tools/v2fix/_stage/v1usage/v1usage.go.golden @@ -0,0 +1,57 @@ +-- gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.Start -- +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2023 Datadog, Inc. + +package main + +import ( + "fmt" + "log" + "os" + + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" +) + +func main() { + // Start the tracer and defer the Stop method. + tracer.Start( // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.Start` + tracer.WithAgentAddr("host:port"), // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.WithAgentAddr` + ) + defer tracer.Stop() // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.Stop` + + // Start a root span. + span := tracer.StartSpan("get.data") // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.StartSpan` + defer span.Finish() // want `\(gopkg.in/DataDog/dd-trace-go.v1/ddtrace.Span\).Finish` + + // Create a child of it, computing the time needed to read a file. + child := tracer.StartSpan( // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.StartSpan` + "read.file", + tracer.ChildOf( // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.ChildOf` + span.Context(), // want `\(gopkg.in/DataDog/dd-trace-go.v1/ddtrace.Span\).Context` + ), + ) + child.SetTag(ext.ResourceName, "test.json") // want `\(gopkg.in/DataDog/dd-trace-go.v1/ddtrace.Span\).SetTag` + + // If you are using 128 bit trace ids and want to generate the high + // order bits, cast the span's context to ddtrace.SpanContextW3C. + // See Issue #1677 + if w3Cctx, ok := child.Context().(ddtrace.SpanContextW3C); ok { // want `\(gopkg.in/DataDog/dd-trace-go.v1/ddtrace.Span\).Context` + fmt.Printf("128 bit trace id = %s\n", w3Cctx.TraceID128()) // want `\(gopkg.in/DataDog/dd-trace-go.v1/ddtrace.SpanContextW3C\).TraceID128` + } + + // Perform an operation. + _, err := os.ReadFile("~/test.json") + + // We may finish the child span using the returned error. If it's + // nil, it will be disregarded. + child.Finish( // want `\(gopkg.in/DataDog/dd-trace-go.v1/ddtrace.Span\).Finish` + tracer.WithError(err), // want `gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.WithError` + ) + if err != nil { + log.Fatal(err) + } +}