2 Commits

Author SHA1 Message Date
Anders Eknert 2378494a23 Modernize fixes and some string building improvements (#8993)
Mostly automated fixes from running:
```
go run golang.org/x/tools/go/analysis/passes/modernize/cmd/modernize@latest --fix ./...
```

But carefully reviewed, and several fixes reverted as they looked like
they potentially could be less performant, and in a few cases due to
bugs in the analyzer that changed semantics of the code. Will report
these upstream.

Mostly good fixes though!

Signed-off-by: Anders Eknert <anders.eknert@apple.com>
2026-08-10 12:49:15 +02:00
rchildress87 81589c1244 vendor a method-less text/template to restore whole-binary linker DCE (#8844)
### Why the changes in this PR are needed?

`text/template`'s field evaluator (`text/template.(*state).evalField`,
`exec.go`) calls`reflect.Value.MethodByName` with a non-constant name.
The Go linker treats a reachable non-constant`MethodByName` as a signal
to disable **method-level dead-code elimination for the whole binary**
(see `cmd/link/internal/ld/deadcode.go` and golang/go#72895). Two OPA
code paths pull stdlib `text/template` into the reachable graph of
ordinary embedders:

1. **Compiler frontend** — `ast.Compiler.Compile → … →
gojsonschema.formatErrorDescription → text/template`. Reached
unconditionally by anything that compiles Rego.
2. **`strings.render_template` builtin** (`v1/topdown/template.go`) —
registered in the topdown builtin table, reachable in anything that
links Rego evaluation.

So an embedder of OPA's compiler/eval retains its entire reachable
method surface — a large binary-size regression, hundreds of MB in the
reporter's case (#7903). Both edges must go before the linker re-enables
method-level DCE for that embedder.

### What are the changes in this PR?

Vendor a self-contained, method-less copy of `text/template` under
`internal/methodlesstemplate` and point both call sites at it. **No
external dependency** (`go.mod`/`go.sum` unchanged).

- Copied verbatim from **Go 1.25.8**: `doc.go`, `exec.go`, `funcs.go`,
`option.go`, `template.go`, plus `internal/fmtsort/sort.go`. Go's BSD
`LICENSE` is preserved in the vendored directory and every file keeps
its `The Go Authors` copyright header.
- Stdlib `text/template/parse` is reused unchanged (the parser has no
`MethodByName`/`evalField` edge, so it does not defeat DCE).
- `helper.go` (`ParseFiles`/`ParseGlob`/`ParseFS`) is dropped — the OPA
call sites only need `New`/`Parse`/`Execute`, and nothing in the kept
files references it.
- **The only edit to the copied code** is removing the `MethodByName`
branch in `exec.go`'s `evalField` (method resolution on the data value).
Everything else is byte-identical, so re-syncing to a newer Go release
is a diff-and-reapply of that single branch removal.
- `internal/gojsonschema` (commit 1) and `v1/topdown` (commit 2) import
the vendored package. The gojsonschema engine is retained in full, so
`ErrorTemplateFuncs` (its `FuncMap` extension point) keeps working —
**no public symbol is removed**.

Rego values and gojsonschema `ErrorDetails` decode to
`map[string]any`/`[]any`/scalars, which have no methods, so removing
method resolution is a provable no-op for these callers.

### Notes to assist PR review:

- **Diff review tip**:
`doc.go`/`funcs.go`/`option.go`/`template.go`/`internal/fmtsort/sort.go`
are **byte-identical** to the Go 1.25.8 originals. Only `exec.go`
differs, in exactly two hunks: the `internal/fmtsort` → vendored import
path, and the removed `MethodByName` block (replaced by a comment
explaining the DCE rationale).
- **Fidelity — render_template**: the `rendertemplate` conformance cases
(incl. `complex` range/if/vars, `simpleint` `%v`, `missingkey` →
`<undefined>`) pass **unchanged**.
- **Fidelity — gojsonschema**: same engine (method-less),
validation-error output unchanged; existing `internal/gojsonschema` and
`v1/ast` tests pass.
- **Tests**: `TestNoStdlibTextTemplateImport` in both
`internal/gojsonschema` and `v1/topdown` scans every non-test file and
asserts none import stdlib `text/template`/`html/template`. `go build
./...`, `go vet ./...` OK; `go mod tidy` is a no-op.
- **Lint**: the vendored directory is added to the golangci-lint path
exclusions, mirroring the existing `internal/gojsonschema` precedent —
the copy is verbatim stdlib, and linting it against OPA's house rules
would force divergence from upstream Go (it trips ~31 stdlib-idiom
issues) and break the diff-and-reapply re-sync.
- **Attribution**: the vendored code is Go stdlib only (BSD, `The Go
Authors`); it contains no third-party/DataDog code.

### Further comments:

- **Scope**: this restores method-level DCE for embedders of OPA's
**compiler/eval**. The standalone `opa` binary additionally links
`v1/server`, which imports `html/template` (a wrapper over
`text/template`) — a separate, independent edge left as a follow-up.
Embedders that don't link the server (the common case) get the full win
from this PR.
- Root cause: golang/go#72895. Closes #7903 for compiler/eval embedders.

---------

Signed-off-by: Dick Childress <dick.childress@icearp.net>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-07 21:08:19 +02:00