diff --git a/README.md b/README.md index 17d8d3a..df3503f 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ Pre-built binaries for Linux, macOS, and Windows are available on the [Releases] ### Requirements -The **target module** — the code you run mutest against — must declare `go 1.20` or later in its `go.mod`. mutest's generated mutation helpers use generics and rely on interfaces satisfying `comparable`, both of which require Go 1.20+. mutest checks this before instrumenting anything and fails fast with a clear error if the module's `go` directive is too old, instead of an obscure compiler error. Modules with no reported Go version (e.g. GOPATH mode) are not checked. +The **target module** — the code you run mutest against — must declare `go 1.20` or later in its `go.mod`; mutest's generated comparison helpers use generics. mutest checks this before instrumenting anything and fails fast with a clear error if the module's `go` directive is too old, instead of an obscure compiler error. Modules with no reported Go version (e.g. GOPATH mode) are not checked. --- @@ -364,7 +364,7 @@ $ mutest -dry-run -json ./... 1. **Parse** — `go/parser` builds an AST from every non-test `.go` file 2. **Discover** — Walk the AST to find `ast.BinaryExpr` with `>`, `>=`, `<`, `<=`, `==`, `!=` (respecting `//mutest:skip`) -3. **Instrument** — Replace each mutation target with a generic helper function call (e.g., `a > b` → `_mutest_cmp_1(a, b)`) and generate a runtime file that switches behavior based on `MUTEST_ID` +3. **Instrument** — Replace each ordered comparison with a generic helper call (e.g., `a > b` → `_mutest_cmp_1(a, b)`) and each equality comparison with a flip of its result (`a == b` → `(a == b) != _mutest_on(1)`; the original comparison stays in place because its operands may legally have different static types, e.g. `any == error`), then generate a runtime file that switches behavior based on `MUTEST_ID` 4. **Build** — Compile one test binary per package with all mutations embedded 5. **Verify baseline** — Run each test binary once with **no** mutation active. If any package's tests fail without a mutation, mutest aborts (a broken or flaky suite would otherwise make every mutant a false KILLED) 6. **Test** — Run the pre-built binary once per mutation with `MUTEST_ID=N`, in a parallel worker pool. Each binary runs with its package directory as the working directory, so `testdata` relative paths resolve diff --git a/cmd/mutest/run_test.go b/cmd/mutest/run_test.go index c43f91d..d5c8ccc 100644 --- a/cmd/mutest/run_test.go +++ b/cmd/mutest/run_test.go @@ -785,11 +785,7 @@ func TestThreshold(t *testing.T) { // TestRun_GoVersionTooOld covers F9: a target module whose go directive is // below 1.20 must fail fast with a clear diagnostic before instrumentation // or build, instead of surfacing a confusing compiler error deep inside -// mutest's generated helpers. mutest's equality helper is instantiated as -// `_mutest_eq_N[T comparable](a, b T)`; comparing two `error` interface -// values (as below) reproduces the "interface satisfies comparable" -// requirement that only compiles under go1.20+, so before this fix the -// build itself fails under go1.19. +// mutest's generated helpers. func TestRun_GoVersionTooOld(t *testing.T) { tmpDir := t.TempDir() writeFiles(t, tmpDir, map[string]string{ @@ -917,3 +913,129 @@ func TestRun_EqualityMutator_Discovered(t *testing.T) { t.Errorf("expected == to != mutation, got %s to %s", points[0].Original, points[0].Mutated) } } + +// TestRun_MixedTypeEquality covers issue #39 end to end: equality operands +// may legally have different static types (e.g. any == error), which the +// old generic equality helper could not infer, aborting the whole package +// at the build stage. The baseline is load-bearing: it fails unless +// instrumentation preserves the original comparison's semantics exactly — +// SamePtr catches any-conversion (equal named/unnamed pointers would +// compare unequal via dynamic types) and Catch catches moving the operands +// into a function literal (recover() inside a nested function no longer +// stops the panic). +func TestRun_MixedTypeEquality(t *testing.T) { + tmpDir := t.TempDir() + writeFiles(t, tmpDir, map[string]string{ + "go.mod": "module example.com/mixedeq\n\ngo 1.21\n", + "lib.go": `package mixedeq + +import ( + "errors" + "io" + "os" +) + +var ErrAbort error = errors.New("abort") + +func IsAbort(r any) bool { return r == ErrAbort } + +func IsStderr(w io.Writer) bool { return w == os.Stderr } + +type Animal interface{ Sound() string } + +type Dog interface { + Animal + Bark() string +} + +func SameAnimal(d Dog, a Animal) bool { return d == a } + +type IntPtr *int + +func SamePtr(p IntPtr, q *int) bool { return p == q } + +func Catch(sentinel any, f func()) (caught bool) { + defer func() { + if recover() == sentinel { + caught = true + } + }() + f() + return +} +`, + "lib_test.go": `package mixedeq + +import ( + "os" + "testing" +) + +type dog struct{} + +func (dog) Sound() string { return "woof" } +func (dog) Bark() string { return "WOOF" } + +type cat struct{} + +func (cat) Sound() string { return "meow" } + +func TestIsAbort(t *testing.T) { + if IsAbort(42) || !IsAbort(ErrAbort) { + t.Fatal("wrong") + } +} + +func TestIsStderr(t *testing.T) { + if !IsStderr(os.Stderr) || IsStderr(os.Stdout) { + t.Fatal("wrong") + } +} + +func TestSameAnimal(t *testing.T) { + if !SameAnimal(dog{}, dog{}) || SameAnimal(dog{}, cat{}) { + t.Fatal("wrong") + } +} + +func TestSamePtr(t *testing.T) { + n, m := 0, 0 + if !SamePtr(&n, &n) || SamePtr(&n, &m) { + t.Fatal("wrong") + } +} + +func TestCatch(t *testing.T) { + if !Catch("boom", func() { panic("boom") }) || Catch("boom", func() {}) { + t.Fatal("wrong") + } +} +`, + }) + + chdir(t, tmpDir) + + var stdout, stderr bytes.Buffer + cfg := config.Config{ + Patterns: []string{"./..."}, + Workers: 2, + Timeout: 30 * time.Second, + JSON: true, + } + + err := run(context.Background(), cfg, &stdout, &stderr) + if err != nil { + t.Fatalf("expected nil error (all mixed-type mutants killed), got %v\nstdout: %s\nstderr: %s", err, stdout.String(), stderr.String()) + } + + var summary output.JSONSummary + if err := json.Unmarshal(stdout.Bytes(), &summary); err != nil { + t.Fatalf("invalid JSON: %v\nstdout: %s", err, stdout.String()) + } + if summary.Total != 5 || summary.Killed != 5 { + t.Errorf("expected total=5 killed=5, got total=%d killed=%d", summary.Total, summary.Killed) + } + if summary.Survived != 0 || summary.Errors != 0 { + t.Errorf("expected survived=0 errors=0, got survived=%d errors=%d", summary.Survived, summary.Errors) + } +} diff --git a/engine/engine.go b/engine/engine.go index 4f14184..ddf61f7 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -37,16 +37,15 @@ type goModule struct { GoVersion string `json:"GoVersion"` } -// minTargetGoVersion is the lowest `go` directive mutest's generated -// mutation helpers support: generics require Go 1.18+, and interfaces -// satisfying the comparable constraint (used for equality helpers, e.g. -// `error` operands) require Go 1.20+. +// minTargetGoVersion is the lowest `go` directive mutest supports in target +// modules: the generated cmp.Ordered helpers require generics (Go 1.18+), +// and 1.20 is mutest's published floor. const minTargetGoVersion = "go1.20" // checkGoVersion fails fast if mod's go directive is older than mutest's // generated helpers require. Without this check, an old go directive -// produces a confusing compiler error deep inside generated code (e.g. -// "error does not satisfy comparable") instead of a clear diagnostic. +// produces a confusing compiler error deep inside generated code instead +// of a clear diagnostic. // // A nil mod, or one with an empty GoVersion (e.g. GOPATH mode, where `go // list -json` reports no Module at all), is skipped: there is nothing to @@ -57,7 +56,7 @@ func checkGoVersion(mod *goModule) error { } found := "go" + mod.GoVersion if version.Compare(found, minTargetGoVersion) < 0 { - return fmt.Errorf("mutest requires the target module's go directive to be >= 1.20 (found go %s in module %s); mutest's generated helpers use generics and interface-satisfies-comparable", mod.GoVersion, mod.Path) + return fmt.Errorf("mutest requires the target module's go directive to be >= 1.20 (found go %s in module %s)", mod.GoVersion, mod.Path) } return nil } diff --git a/engine/engine_test.go b/engine/engine_test.go index c0cd86f..61cc284 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -430,9 +430,8 @@ func RuntimeCheck(n int) bool { return n > 5 } } // TestCheckGoVersion covers F9: the target module's go directive must be -// >= 1.20 (mutest's generated helpers use generics and interfaces -// satisfying comparable, both of which require Go 1.20+). A module with no -// reported GoVersion (e.g. GOPATH mode) must be skipped, not rejected. +// >= 1.20, mutest's published floor. A module with no reported GoVersion +// (e.g. GOPATH mode) must be skipped, not rejected. func TestCheckGoVersion(t *testing.T) { tests := []struct { name string diff --git a/engine/instrument.go b/engine/instrument.go index d483e42..8f16368 100644 --- a/engine/instrument.go +++ b/engine/instrument.go @@ -30,10 +30,9 @@ type InstrumentedPackage struct { OverlayPath string } -// helperSpec describes a generated mutation helper function. +// helperSpec describes a generated _mutest_cmp_N helper function. type helperSpec struct { ID int - Kind string // "cmp" (cmp.Ordered), "eq" (comparable), or "inline" (nil comparisons) Original token.Token Mutated token.Token } @@ -170,15 +169,13 @@ type mutTarget struct { xStart, xEnd int // byte offsets in original source for LHS operand yStart, yEnd int // byte offsets in original source for RHS operand point mutator.MutationPoint - isNil bool // true when one operand is nil (uses inline func) - kind string } // fullStart/fullEnd returns the byte range of the entire binary expression. func (t *mutTarget) fullStart() int { return t.xStart } func (t *mutTarget) fullEnd() int { return t.yEnd } -// instrumentFile replaces mutation target expressions with helper function calls. +// instrumentFile replaces mutation target expressions with instrumented forms. // Nested binary expressions (e.g., `(a > b) == flag`) are handled by building // replacement text bottom-up: inner replacements are embedded in outer ones. func instrumentFile(src []byte, filePath string, points []mutator.MutationPoint) ([]byte, []helperSpec, error) { @@ -207,18 +204,12 @@ func instrumentFile(src []byte, filePath string, points []mutator.MutationPoint) key := nodeKey{nodeID, bin.Op} if pt, exists := pointByKey[key]; exists { - kind := "cmp" - if pt.Original == token.EQL || pt.Original == token.NEQ { - kind = "eq" - } targets = append(targets, mutTarget{ xStart: fset.Position(bin.X.Pos()).Offset, xEnd: fset.Position(bin.X.End()).Offset, yStart: fset.Position(bin.Y.Pos()).Offset, yEnd: fset.Position(bin.Y.End()).Offset, point: pt, - isNil: isNilIdent(bin.X) || isNilIdent(bin.Y), - kind: kind, }) } @@ -226,6 +217,12 @@ func instrumentFile(src []byte, filePath string, points []mutator.MutationPoint) return true }) + // A point that matched no AST node would be scheduled by the runner as a + // no-op mutant and silently reported SURVIVED; fail loudly instead. + if len(targets) != len(points) { + return nil, nil, fmt.Errorf("only %d of %d mutation points matched an AST node", len(targets), len(points)) + } + // Phase 2: Build replacement text bottom-up (innermost first). // Sort by range size ascending so inner targets are processed first. sort.Slice(targets, func(i, j int) bool { @@ -248,14 +245,23 @@ func instrumentFile(src []byte, filePath string, points []mutator.MutationPoint) rhs := textWithInnerRepls(src, t.yStart, t.yEnd, builtRepls) var callExpr string - if t.isNil { - callExpr = fmt.Sprintf("func() bool { _mutest_init(); if _mutest_active == %d { return %s %s %s }; return %s %s %s }()", - pt.MutestID, lhs, pt.Mutated.String(), rhs, lhs, pt.Original.String(), rhs) - helpers = append(helpers, helperSpec{ID: pt.MutestID, Kind: "inline", Original: pt.Original, Mutated: pt.Mutated}) + if pt.Original == token.EQL || pt.Original == token.NEQ { + // Equality operands may legally have different static types + // (e.g. `any == error`), which a one-type-parameter generic + // helper cannot infer, and moving them into a helper or closure + // would change meaning: `any`-boxing makes named-vs-unnamed + // concrete types compare unequal, and recover() stops a panic + // only when called directly by a deferred function. Since the + // mutation is exactly a negation, XOR the original comparison — + // left verbatim in place, evaluated once — with the mutation + // switch. A comparison also yields an untyped bool, so + // defined-bool-type contexts keep compiling. + callExpr = fmt.Sprintf("(%s %s %s) != _mutest_on(%d)", lhs, pt.Original.String(), rhs, pt.MutestID) } else { - funcName := fmt.Sprintf("_mutest_%s_%d", t.kind, pt.MutestID) - callExpr = fmt.Sprintf("%s(%s, %s)", funcName, lhs, rhs) - helpers = append(helpers, helperSpec{ID: pt.MutestID, Kind: t.kind, Original: pt.Original, Mutated: pt.Mutated}) + // Ordered mutations are not negations (`>` vs `>=`), so they go + // through a generic helper instead. + callExpr = fmt.Sprintf("_mutest_cmp_%d(%s, %s)", pt.MutestID, lhs, rhs) + helpers = append(helpers, helperSpec{ID: pt.MutestID, Original: pt.Original, Mutated: pt.Mutated}) } builtRepls[[2]int{t.fullStart(), t.fullEnd()}] = replacement{ @@ -347,28 +353,14 @@ func textWithInnerRepls(src []byte, start, end int, built map[[2]int]replacement return buf.String() } -// isNilIdent returns true if the expression is the identifier "nil". -func isNilIdent(expr ast.Expr) bool { - ident, ok := expr.(*ast.Ident) - return ok && ident.Name == "nil" -} - // generateRuntime generates the mutest_runtime.go file content. func generateRuntime(pkg string, helpers []helperSpec) []byte { var b strings.Builder b.WriteString("package " + pkg + "\n\n") - needsCmp := false - for _, h := range helpers { - if h.Kind == "cmp" { - needsCmp = true - break - } - } - b.WriteString("import (\n") - if needsCmp { + if len(helpers) > 0 { b.WriteString("\t\"cmp\"\n") } b.WriteString("\t\"os\"\n") @@ -388,18 +380,16 @@ func generateRuntime(pkg string, helpers []helperSpec) []byte { b.WriteString("\t\t\t_mutest_active, _ = strconv.Atoi(s)\n") b.WriteString("\t\t}\n") b.WriteString("\t})\n") + b.WriteString("}\n\n") + + b.WriteString("func _mutest_on(id int) bool {\n") + b.WriteString("\t_mutest_init()\n") + b.WriteString("\treturn _mutest_active == id\n") b.WriteString("}\n") for _, h := range helpers { - if h.Kind == "inline" { - continue - } b.WriteString("\n") - if h.Kind == "cmp" { - fmt.Fprintf(&b, "func _mutest_cmp_%d[T cmp.Ordered](a, b T) bool {\n", h.ID) - } else { - fmt.Fprintf(&b, "func _mutest_eq_%d[T comparable](a, b T) bool {\n", h.ID) - } + fmt.Fprintf(&b, "func _mutest_cmp_%d[T cmp.Ordered](a, b T) bool {\n", h.ID) b.WriteString("\t_mutest_init()\n") fmt.Fprintf(&b, "\tif _mutest_active == %d {\n", h.ID) fmt.Fprintf(&b, "\t\treturn a %s b\n", h.Mutated.String()) diff --git a/engine/instrument_test.go b/engine/instrument_test.go index 6b9a8ff..f23041d 100644 --- a/engine/instrument_test.go +++ b/engine/instrument_test.go @@ -2,6 +2,7 @@ package engine import ( "context" + "go/parser" "go/token" "os" "path/filepath" @@ -11,6 +12,13 @@ import ( "github.com/fchimpan/mutest/mutator" ) +func mustParseInstrumented(t *testing.T, out []byte) { + t.Helper() + if _, err := parser.ParseFile(token.NewFileSet(), "instrumented.go", out, 0); err != nil { + t.Fatalf("instrumented output does not parse: %v\n%s", err, out) + } +} + func TestInstrumentFile_NestedBinaryExpr(t *testing.T) { // (a > b) == flag: both > and == should be instrumented. // The inner > call is embedded in the outer == call's LHS. @@ -49,23 +57,16 @@ func Foo(a, b int, flag bool) bool { } result := string(out) + mustParseInstrumented(t, out) - // Both mutations should be instrumented. - if !strings.Contains(result, "_mutest_eq_2") { - t.Errorf("expected outer == to be instrumented, got:\n%s", result) - } - if !strings.Contains(result, "_mutest_cmp_1") { - t.Errorf("expected inner > to be instrumented (nested in outer), got:\n%s", result) + // Both mutations should be instrumented, the inner > call nested inside + // the outer == flip. + if !strings.Contains(result, "((_mutest_cmp_1(a, b)) == flag) != _mutest_on(2)") { + t.Errorf("expected inner > call nested in outer == flip, got:\n%s", result) } - // The inner call should appear inside the outer call's LHS argument. - if !strings.Contains(result, "_mutest_eq_2(_mutest_cmp_1(a, b), flag)") && - !strings.Contains(result, "_mutest_eq_2((_mutest_cmp_1(a, b)), flag)") { - t.Errorf("expected inner > call nested in outer == call, got:\n%s", result) - } - - if len(helpers) != 2 { - t.Errorf("expected 2 helpers, got %d", len(helpers)) + if len(helpers) != 1 { + t.Errorf("expected 1 cmp helper, got %d", len(helpers)) } } @@ -116,22 +117,16 @@ func Bar(x, y *int) bool { } result := string(out) + mustParseInstrumented(t, out) - // All three should be present. - // Inner != are nil comparisons → inline funcs with _mutest_active == N. - if !strings.Contains(result, "_mutest_active == 1") { - t.Errorf("expected left != (ID=1) to be instrumented, got:\n%s", result) - } - if !strings.Contains(result, "_mutest_active == 3") { - t.Errorf("expected right != (ID=3) to be instrumented, got:\n%s", result) - } - // Outer == is a non-nil comparison → helper func _mutest_eq_2. - if !strings.Contains(result, "_mutest_eq_2") { - t.Errorf("expected outer == (ID=2) to be instrumented as _mutest_eq_2, got:\n%s", result) + // All three flips should be present, each ID bound to its own site. + want := "(((x != nil) != _mutest_on(1)) == ((y != nil) != _mutest_on(3))) != _mutest_on(2)" + if !strings.Contains(result, want) { + t.Errorf("expected %s, got:\n%s", want, result) } - if len(helpers) != 3 { - t.Errorf("expected 3 helpers, got %d", len(helpers)) + if len(helpers) != 0 { + t.Errorf("expected 0 cmp helpers, got %d", len(helpers)) } } @@ -181,26 +176,119 @@ func Baz(a, b int, flag, expected bool) bool { } result := string(out) + mustParseInstrumented(t, out) + + // All three mutations should be present, nested innermost to outermost. + want := "((((_mutest_cmp_1(a, b)) == flag) != _mutest_on(2)) != expected) != _mutest_on(3)" + if !strings.Contains(result, want) { + t.Errorf("expected %s, got:\n%s", want, result) + } + + if len(helpers) != 1 { + t.Errorf("expected 1 cmp helper, got %d", len(helpers)) + } +} + +// TestInstrumentFile_MixedTypeEquality covers issue #39: `any == error` is +// legal Go, but a one-type-parameter generic helper cannot infer T from +// operands of different static types, so the instrumented package failed +// to build. The flip form keeps the original comparison untouched. +func TestInstrumentFile_MixedTypeEquality(t *testing.T) { + src := []byte(`package repro + +import "net/http" + +func IsAbort(r any) bool { + return r == http.ErrAbortHandler +} +`) + + points := []mutator.MutationPoint{ + { + File: "repro.go", + Package: "repro", + NodeID: 0, + Original: token.EQL, + Mutated: token.NEQ, + MutestID: 1, + Desc: "== to !=", + }, + } + + out, helpers, err := instrumentFile(src, "repro.go", points) + if err != nil { + t.Fatalf("instrumentFile: %v", err) + } + + result := string(out) + mustParseInstrumented(t, out) - // All three mutations should be present. - if !strings.Contains(result, "_mutest_cmp_1") { - t.Errorf("expected innermost > to be instrumented, got:\n%s", result) + if strings.Contains(result, "_mutest_eq_") { + t.Errorf("equality must not use a generic helper (breaks mixed-type inference), got:\n%s", result) + } + if !strings.Contains(result, "(r == http.ErrAbortHandler) != _mutest_on(1)") { + t.Errorf("expected flip preserving the original comparison, got:\n%s", result) } - if !strings.Contains(result, "_mutest_eq_2") { - t.Errorf("expected middle == to be instrumented, got:\n%s", result) + if len(helpers) != 0 { + t.Errorf("expected 0 cmp helpers, got %+v", helpers) + } +} + +// TestInstrumentFile_RecoverOperand pins the flip form for position-sensitive +// operands: recover() stops a panic only when called directly by a deferred +// function, so instrumentation must not move the comparison into a nested +// function literal. +func TestInstrumentFile_RecoverOperand(t *testing.T) { + src := []byte(`package repro + +func Catch(sentinel any, f func()) (caught bool) { + defer func() { + if recover() == sentinel { + caught = true + } + }() + f() + return +} +`) + + points := []mutator.MutationPoint{ + { + File: "repro.go", + Package: "repro", + NodeID: 0, + Original: token.EQL, + Mutated: token.NEQ, + MutestID: 1, + Desc: "== to !=", + }, } - if !strings.Contains(result, "_mutest_eq_3") { - t.Errorf("expected outermost != to be instrumented, got:\n%s", result) + + out, _, err := instrumentFile(src, "repro.go", points) + if err != nil { + t.Fatalf("instrumentFile: %v", err) } - // Verify nesting: cmp_1 inside eq_2 inside eq_3 - if !strings.Contains(result, "_mutest_eq_2(_mutest_cmp_1(a, b), flag)") && - !strings.Contains(result, "_mutest_eq_2((_mutest_cmp_1(a, b)), flag)") { - t.Errorf("expected cmp_1 nested inside eq_2, got:\n%s", result) + result := string(out) + mustParseInstrumented(t, out) + + if !strings.Contains(result, "if (recover() == sentinel) != _mutest_on(1) {") { + t.Errorf("expected recover() to stay in the deferred function's frame, got:\n%s", result) } +} - if len(helpers) != 3 { - t.Errorf("expected 3 helpers, got %d", len(helpers)) +// TestGenerateRuntime_MutationSwitch pins the two runtime properties call +// sites depend on: _mutest_on must read MUTEST_ID via _mutest_init before +// comparing, and cmp must not be imported without cmp helpers (an unused +// import would fail the build of every equality-only package). +func TestGenerateRuntime_MutationSwitch(t *testing.T) { + runtime := string(generateRuntime("repro", nil)) + + if !strings.Contains(runtime, "func _mutest_on(id int) bool {\n\t_mutest_init()\n\treturn _mutest_active == id\n}") { + t.Errorf("expected initializing _mutest_on, got:\n%s", runtime) + } + if strings.Contains(runtime, `"cmp"`) { + t.Errorf("cmp must not be imported without cmp helpers, got:\n%s", runtime) } }