A helper returns a nil *ValidationError as an error and the caller's check fires - what is the fix?

## What the symptom looks like An interactive setup wizard walks the user through fields one at a time - hostname, port, email - and each step calls a validator. The validator was written like this: ```go func validateEmail(s string) *ValidationError { if strings.Contains(s, "@") { return nil } return &ValidationError{Field: "email"} } ``` and the step runner like this: ```go func runStep(s string) error { return validateEmail(s) } ``` Every step now fails. A perfectly valid address prints "step failed" with no message, or worse, the program prints the error and panics dereferencing it. The validator did return nil, the caller did check `err != nil`, and the check still fired. ## Why (stated, not re-derived) An `error` variable is an interface value, and an interface value is only nil when it holds no concrete type at all. Converting a `*ValidationError` - even a nil one - into an `error` produces an interface that names `*ValidationError` and carries a nil pointer. It is non-nil by definition. In a debugger this is unmissable once you know to look: stop on the caller's `err`, expand it, and you see a type word reading `*main.ValidationError` beside a value word of `0x0`. The two halves disagree, and the comparison follows the first one. ## The fix is in the signature The conversion happens because the helper's *declared result type* is concrete. Remove that and the trap cannot occur: ```go func validateEmail(s string) error { if !strings.Contains(s, "@") { return &ValidationError{Field: "email"} } return nil } ``` Now the success path returns a literal `nil` of interface type, and the only value that ever reaches an `error` variable is one that was actually constructed. Note the shape: the concrete value is built *inside* the failure branch, and the last statement is a bare `return nil`. That ordering is not cosmetic - it is what makes the bug unrepresentable. ## The variants a reviewer should watch for Changing the obvious signature is not enough on its own, because the same conversion can happen anywhere a concrete error type is assigned into an interface. In a pull request, look for: - **Any function whose result type is a concrete error type.** `func check(...) *ValidationError` is the headline case, and it is worth treating as a blanket rule: functions return `error`, full stop. - **A named result of concrete type.** `func check(...) (e *ValidationError)` with a bare `return` at the end has the same problem the moment the value is adapted to `error`. - **A struct field or slice element typed concretely.** `type stepResult struct { err *ValidationError }` looks harmless until someone writes `return r.err` from a function returning `error`. - **A local declared with `var e *ValidationError`** that is returned at the end of a function whose result type is `error`. This is the sneakiest form, because the signature looks correct. - **An interface-typed value passed on.** Once the value is already an `error`, passing it along is safe; the danger is only at the concrete-to-interface boundary. ## What is *not* the fix Several tempting repairs make things worse: - **Changing the receiver on `Error()`** from pointer to value changes nothing about the trap; it only changes which types satisfy the interface. - **Making callers type-assert before checking** pushes the producer's mistake onto every caller, forever, and it will be forgotten at the one call site that matters. - **Returning an empty `&ValidationError{}` instead of nil** turns "always fails" into "always fails with an empty message", which is harder to spot. - **Comparing `err.Error() == ""`** builds a second, string-based error protocol on top of a broken one. ## Why review has to catch it This compiles cleanly, and the standard `go vet` suite has no analyser for it. There is no runtime warning either - the program simply behaves as though everything failed. It is exactly the class of defect that survives unit tests written against the same helper, because a test that asserts "invalid input produces an error" passes; only a test asserting that *valid* input produces a nil error catches it. That test is cheap and worth writing for any validator. ## The rule to carry away A function's error result is declared `error`. The concrete type is an implementation detail of the failure path: construct it where the failure happens, return it as `error`, and let callers recover the concrete type only through the machinery designed for that. Anything else exports the risk that a successful call looks like a failed one - and the wizard that tells a user their correct email is invalid is the friendliest possible version of that bug. In an RPC handler or a payment path, it is a silent outage.