fix: stop table-driven tests from being reported as clones

README.md

@@ -295,6 +295,12 @@ dupehound tokenizes source files into a normalized token stream (stripping comme
 
 Detection is restricted to **function and method bodies** — imports, top-level declarations, struct definitions, config blocks, and keyword maps are automatically excluded. This dramatically reduces false positives and focuses on actionable logic duplication.
 
+**Data tables inside functions are excluded too.** The rows of a collection literal are structurally identical by design, so a clone whose every instance falls inside a single literal is reported as data, not duplication — the table-driven test pattern no longer flags itself.
+
+This covers Go (`[]T{…}`, `map[K]V{…}`, `[]struct{…}{…}`), Python (lists and dicts), JavaScript/TypeScript, Ruby, Rust, PHP, Elixir, Swift, Dart, and Lua. Java, C, C++, C# and Kotlin are deliberately left out: there `[` is an index or an array type and `{` is also a block, so a literal cannot be told from code by tokens alone.
+
+Logic still counts. A callback column (`run func(t *testing.T)`, `run: () => {…}`) is checked normally, two identical tables in different places are still flagged, and a short inline `[]T{a, b}` never breaks up the block around it.
+
 ## Output formats
 
 - **text** — human-readable with hotspots, top clones by impact, and full clone list

docs/specs/001-table-literal-false-positive.plan.md

@@ -0,0 +1,138 @@
+# Plan: Fix table-driven test literal false positives (issue #31)
+
+**Spec of record:** [GitHub issue #31](https://github.com/AxeForging/dupehound/issues/31) —
+no `docs/specs/` spec exists for this; the issue contains the full repro, expected
+behavior, and suggested fixes, so it serves as the spec (spec review waived as a bugfix).
+
+**Problem:** Idiomatic Go table-driven tests (`tests := []struct{…}{…}` inside a test
+function) are reported as type-2 clones. The reported "instances" are overlapping
+sliding windows over the rows of a single composite literal. This contradicts the
+README's documented exclusion of data/config literals — the exclusion only works
+today because struct/config blocks usually sit *outside* function bodies, and
+`markFunctionBodies` masking is the only data filter.
+
+**Root causes (two independent defects):**
+
+1. `services/detector.go` `detectExact`: seed windows are spaced `>= minTokens`
+   apart, but greedy extension grows each block to `totalTokens > minTokens`, so
+   emitted instances of one clone group overlap each other. Overlapping windows are
+   never distinct duplicates.
+2. There is no notion of composite literals *inside* function bodies, so a data
+   table is treated as logic. Even with defect 1 fixed, a uniform 10-row table still
+   yields disjoint identical windows and gets flagged.
+
+## Design change during implementation
+
+The plan originally chose the issue's suggestion **1** (exclude literal tokens from
+detection, by clearing `InFunc`). That was implemented, then **rejected on evidence**
+and replaced with the issue's suggestion **2** (suppress clone groups confined to one
+literal).
+
+Why: masking tokens punches a hole through the middle of surrounding code. A short
+inline `[]TokenizedFile{a, b}` sitting inside an otherwise-duplicated block split that
+block in two and the finding was lost. Measured on this repo's own test suite, the
+masking approach removed **58 clones, ~50 of them genuine** duplication. The
+span-based rule removes **5, of which 4 are overlapping-window artifacts and 1 is a
+real data table** — with zero new findings and no real duplication lost.
+
+Final design:
+
+- **Overlap guard** (`dropOverlappingStarts`): after extension, same-file instances
+  must be spaced `>= totalTokens`; groups left with < 2 instances are dropped.
+- **Data-literal spans** (`findDataLiteralSpans`): composite literals are recorded as
+  token spans on `TokenizedFile`, *not* excluded from detection. `detectExact` drops a
+  clone group when every instance lies inside one span; `detectFuzzy` skips candidate
+  pairs whose blocks share a span (resolved once per block, not per pair).
+- **Func-literal exemption** (`findFuncLiteralBodies`): a window touching a function
+  literal body is logic, never data — so a `run func(t *testing.T)` column is still
+  checked. Distinguishing a func literal from a struct field's func *type* needed its
+  own parse (`goFuncLiteralBody`); `markFunctionBodies` keys off the `func` keyword
+  alone and cannot tell them apart.
+
+## Steps
+
+- [x] **TDD repro first (must fail):** detector-level test reproducing issue #31
+      verbatim, plus a gofmt-style multi-line-row table.
+      Files: `services/table_literal_test.go`.
+- [x] **Overlap guard in `detectExact`.** Files: `services/detector.go`,
+      `services/overlap_test.go`.
+- [x] **Data-literal span discovery + suppression rule**, wired into both the exact
+      and fuzzy detectors. Files: `services/tokenizer.go`, `services/detector.go`,
+      `services/composite_literal_test.go`, `services/testhelpers_test.go`.
+- [x] **Extend beyond Go** to every language whose collection literals are
+      unambiguous, with per-language positive, negative (indexing) and callback tests.
+      Files: `services/tokenizer.go`, `services/data_literal_langs_test.go`.
+- [x] **Fix the bugs surfaced along the way** (see below): Python function
+      boundaries, Dart body misclassification, nil-language panic.
+      Files: `services/tokenizer.go`, `services/pyfuncboundary_test.go`.
+- [x] **Repro green + full regression:** `go test -race ./...` (469 pass),
+      `make lint` (0 issues), `gofumpt -w`. No golden-file churn.
+- [x] **Integration tests via the built binary.** Files:
+      `integration/table_literal_test.go`. All four fail on `main`, pass on the branch.
+- [x] **Docs:** README "How it works" documents the data-table exclusion and its
+      limits. Files: `README.md`.
+- [x] **Branch + PR:** `fix/table-literal-false-positive`, `Closes #31`.
+
+## Verification performed
+
+- Every new test was run against pristine `main` and **fails** there (4/4 integration,
+  4/6 unit — the 2 that pass on main are the negative "real duplication is still
+  reported" guards, which must pass in both).
+- A/B scan of identical pristine `main` source with the pre-fix and post-fix binaries:
+  157 → 152 clones, 0 new, and the only non-overlap removal is a genuine
+  `[]domain.Clone{…}` fixture table.
+- Self-scan under the repo's own `.dupehound.yml` is byte-identical before and after
+  (13 clones, 5.1%), so the CI quality gate is unaffected.
+
+## Risks
+
+- **Heuristic misclassification:** token-level Go brace disambiguation is heuristic.
+  Mitigated by a conservative trigger (only `]T{` / `map[…]V{` / `]struct{…}{`),
+  Go-only gating, and negative tests covering `if m[k] {`, range statements, index
+  expressions on call results, func literals, and plain `T{…}` struct literals.
+- **Under-suppression is the deliberate failure mode:** anything ambiguous stays
+  reported. A missed false positive is noise; a swallowed real clone is a silent
+  correctness loss.
+- **Baselines:** users with committed baselines may see previously reported table
+  clones disappear. Stale entries are inert, not breaking.
+- **Rollback:** revert the PR — no schema, config, or flag changes; behavior-only.
+
+## Done means
+
+- The exact repro from issue #31 scans to `Clones found: 0` (unit + binary-level).
+- No clone group ever reports overlapping same-file instances (unit-tested invariant).
+- Genuine duplication is still detected: in test files, in table func-literal columns,
+  and in blocks containing short inline literals — each covered by a dedicated test.
+- `go test -race ./...` and `make lint` clean; PR open referencing issue #31.
+
+## Bugs found while working — all fixed in this PR
+
+1. **Python function boundaries (`markPythonFunctions`)** — the body scan ended at the
+   next blank-line-separated `def`, then resumed the outer scan *after* that keyword,
+   stepping over it. Every definition following the first was never marked as a
+   function body, and since detection only looks inside function bodies, all
+   duplication in those functions was invisible. In a file of `n` blank-line separated
+   defs, only the first was ever scanned. Fixed by resuming *on* the boundary keyword.
+   Regression tests: `services/pyfuncboundary_test.go` (7 of 8 fail on `main`).
+2. **Dart function bodies misread as literals** — bodies in languages without a `func`
+   keyword come from a brace-depth heuristic that cannot tell a function body from a
+   `{…}` map literal at the same depth, so every row of a Dart table looked like a
+   function body. Fixed with `isRealFunctionBody`: a brace-delimited body follows `)`
+   or an arrow, a data literal follows `=`, `,` or `[`.
+3. **`TokenizeFile` nil-language panic** — it dereferenced `lang.Name` while
+   `markFunctionBodies` explicitly documents nil as "no syntax knowledge". Unreachable
+   today (`collectFiles` drops unknown-language files first), but the contract was
+   incoherent. Fixed by substituting a zero-value language.
+
+## Language coverage
+
+The rule applies wherever a collection literal can be told from code by tokens alone:
+
+- **Go** — typed composite literals: `[]T{…}`, `map[K]V{…}`, `[]struct{…}{…}`.
+- **Bracket literals** (`[…]` in expression position) — Python, JavaScript, TypeScript,
+  Ruby, Rust, PHP, Elixir, Swift, Dart.
+- **Brace literals** (`{…}` where braces are never blocks) — Python, Lua, Elixir.
+
+Deliberately excluded: Java, C, C++, C#, Kotlin (there `[` is an index or array type
+and `{` is also a block) and Scala (`[` is type parameters). Ruby takes brackets only,
+since `{` is either a hash or a block argument.

integration/table_literal_test.go

@@ -0,0 +1,208 @@
+package integration
+
+import (
+	"encoding/json"
+	"os/exec"
+	"testing"
+
+	"github.com/AxeForging/dupehound/domain"
+)
+
+// End-to-end coverage for issue #31, driven through the built binary at its
+// real defaults. The unit tests pin the mask and the overlap guard; these pin
+// what a user actually sees when they run `dupehound scan` on a Go package with
+// table-driven tests.
+
+// issue31Repro is the fixture from the issue report.
+const issue31Repro = `package repro
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestRedact(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   string
+		absent  string
+		present string
+	}{
+		{name: "openai preset case", input: "token openai_AAAA0\n", absent: "openai_AAAA", present: "openai_***"},
+		{name: "anthropic preset case", input: "token anthropic_AAAA1\n", absent: "anthropic_AAAA", present: "anthropic_***"},
+		{name: "github preset case", input: "token github_AAAA2\n", absent: "github_AAAA", present: "github_***"},
+		{name: "awskey preset case", input: "token awskey_AAAA3\n", absent: "awskey_AAAA", present: "awskey_***"},
+		{name: "bearer preset case", input: "token bearer_AAAA4\n", absent: "bearer_AAAA", present: "bearer_***"},
+		{name: "stripe preset case", input: "token stripe_AAAA5\n", absent: "stripe_AAAA", present: "stripe_***"},
+		{name: "gcp preset case", input: "token gcp_AAAA6\n", absent: "gcp_AAAA", present: "gcp_***"},
+		{name: "azure preset case", input: "token azure_AAAA7\n", absent: "azure_AAAA", present: "azure_***"},
+		{name: "npm preset case", input: "token npm_AAAA8\n", absent: "npm_AAAA", present: "npm_***"},
+		{name: "slack preset case", input: "token slack_AAAA9\n", absent: "slack_AAAA", present: "slack_***"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if strings.Contains(tc.present, tc.absent) {
+				t.Errorf("%s", tc.name)
+			}
+		})
+	}
+}
+`
+
+// neutralConfig writes a config with no excludes into dir and returns its path.
+//
+// Config discovery walks up from the working directory, so a scan launched from
+// the test binary would otherwise inherit dupehound's own .dupehound.yml — which
+// excludes *_test.go and would silently skip every fixture here. Passing an
+// explicit config pins these tests to default scanning behaviour, which is the
+// behaviour issue #31 is about.
+func neutralConfig(t *testing.T, dir string) string {
+	t.Helper()
+	return writeFile(t, dir, "neutral.yml", "scan:\n  exclude: []\n")
+}
+
+// scanJSON runs a scan over dir and decodes the report.
+func scanJSON(t *testing.T, bin, dir string, args ...string) domain.Report {
+	t.Helper()
+	full := append([]string{
+		"scan", "--path", dir,
+		"--config", neutralConfig(t, dir),
+		"--format", "json", "--exit-zero",
+	}, args...)
+	out, err := exec.Command(bin, full...).Output() // stdout only — logs go to stderr
+	if err != nil {
+		t.Fatalf("scan failed: %v", err)
+	}
+	var report domain.Report
+	if err := json.Unmarshal(out, &report); err != nil {
+		t.Fatalf("invalid JSON output: %v\n%s", err, string(out))
+	}
+	return report
+}
+
+// scanExitCode runs a scan for its exit status — the signal CI and pre-commit
+// hooks actually gate on.
+func scanExitCode(t *testing.T, bin, dir string) error {
+	t.Helper()
+	return exec.Command(bin, "scan", "--path", dir,
+		"--config", neutralConfig(t, dir), "--quiet").Run()
+}
+
+func TestScan_Issue31_TableDrivenTestIsClean(t *testing.T) {
+	bin := buildBinary(t)
+	dir := t.TempDir()
+	writeFile(t, dir, "table_test.go", issue31Repro)
+
+	report := scanJSON(t, bin, dir)
+
+	if len(report.Clones) != 0 {
+		t.Errorf("table-driven test reported %d clone(s); expected a clean scan", len(report.Clones))
+		for _, c := range report.Clones {
+			t.Logf("  %s similarity=%.2f instances=%d", c.Type, c.Similarity, len(c.Instances))
+			for _, in := range c.Instances {
+				t.Logf("    %s:%d-%d", in.File, in.StartLine, in.EndLine)
+			}
+		}
+	}
+}
+
+// The scan must exit 0 for a package whose only "duplication" was a data table
+// — that exit code is what gates CI and pre-commit hooks.
+func TestScan_Issue31_TableDrivenTestExitsZeroWithoutExitZeroFlag(t *testing.T) {
+	bin := buildBinary(t)
+	dir := t.TempDir()
+	writeFile(t, dir, "table_test.go", issue31Repro)
+
+	// Deliberately without --exit-zero: a clean scan must exit 0 on its own.
+	if err := scanExitCode(t, bin, dir); err != nil {
+		t.Errorf("scan exited non-zero on a package with only a data table: %v", err)
+	}
+}
+
+// The counterpart guard: genuinely duplicated logic in the same package is
+// still found, and the scan still fails. Without this, "no clones" could just
+// mean detection is broken.
+func TestScan_Issue31_RealDuplicationStillFailsTheScan(t *testing.T) {
+	bin := buildBinary(t)
+	dir := t.TempDir()
+	writeFile(t, dir, "table_test.go", issue31Repro)
+
+	dup := `
+	server := newServer()
+	defer server.Close()
+	client := server.Client()
+	req := buildRequest(server.URL)
+	resp, err := client.Do(req)
+	if err != nil {
+		panic(err)
+	}
+	defer resp.Body.Close()
+	body := readAll(resp.Body)
+	if resp.StatusCode != 200 {
+		panic(body)
+	}
+	record(body)
+`
+	writeFile(t, dir, "handlers.go",
+		"package repro\n\nfunc alpha() {"+dup+"}\n\nfunc bravo() {"+dup+"}\n")
+
+	report := scanJSON(t, bin, dir)
+	if len(report.Clones) == 0 {
+		t.Fatal("copy-pasted function bodies must still be reported")
+	}
+
+	// Every reported instance must come from the real duplication, not the table.
+	for _, c := range report.Clones {
+		for _, in := range c.Instances {
+			if in.File == "table_test.go" || filepathBase(in.File) == "table_test.go" {
+				t.Errorf("clone instance points at the data table: %s:%d-%d", in.File, in.StartLine, in.EndLine)
+			}
+		}
+	}
+
+	// And a scan with real duplication must fail (exit non-zero) so CI blocks it.
+	if err := scanExitCode(t, bin, dir); err == nil {
+		t.Error("scan exited 0 despite genuine duplication; CI would not block it")
+	}
+}
+
+// No clone group may ever report overlapping instances within one file.
+func TestScan_Issue31_NoOverlappingInstancesInReport(t *testing.T) {
+	bin := buildBinary(t)
+	dir := t.TempDir()
+
+	// A long, highly periodic body — the shape that produced sliding-window
+	// "instances" of the same text before the overlap guard.
+	src := "package repro\n\nfunc run() {\n"
+	for i := 0; i < 40; i++ {
+		src += "\tstep(a, b)\n\tcheck(a, b)\n\temit(a, b)\n"
+	}
+	src += "}\n"
+	writeFile(t, dir, "periodic.go", src)
+
+	report := scanJSON(t, bin, dir)
+	for _, c := range report.Clones {
+		for i := range c.Instances {
+			for j := i + 1; j < len(c.Instances); j++ {
+				a, b := c.Instances[i], c.Instances[j]
+				if a.File != b.File {
+					continue
+				}
+				if a.StartLine <= b.EndLine && b.StartLine <= a.EndLine {
+					t.Errorf("clone %s reports overlapping instances in %s: %d-%d and %d-%d",
+						c.Hash, a.File, a.StartLine, a.EndLine, b.StartLine, b.EndLine)
+				}
+			}
+		}
+	}
+}
+
+// filepathBase avoids importing path/filepath just for one call in assertions.
+func filepathBase(p string) string {
+	for i := len(p) - 1; i >= 0; i-- {
+		if p[i] == '/' || p[i] == '\\' {
+			return p[i+1:]
+		}
+	}
+	return p
+}