A Go function returns a nil error even though an inner if block set err — how do you diagnose and prevent this class of bug?

## What actually happened A short variable declaration declares into the innermost enclosing block. Inside an `if`, a `for` body or a `select` case, `err := doSomething()` therefore creates a *second* variable named `err` whose life ends at the closing brace. The outer variable — the one the function returns — was never written, so it still holds its zero value, `nil`. Take a patch tool that applies hunks to a file: ```go func apply(hunks []hunk) error { var err error for _, h := range hunks { if h.reversed { err := writeReversed(h) // := declares a new err in this block if err != nil { continue // the outer err never sees this failure } } else { err = writeHunk(h) } } return err } ``` Every reversed hunk that fails is silently dropped, and the tool reports a clean apply over a file it did not finish patching. Nothing in the build complains: shadowing is legal, and the unused-variable rule is satisfied because the inner `err` is checked inside its own block. ## Diagnosing the instance 1. **Localise by value, not by control flow.** The returned value is a zero value, which means the returning variable was never assigned on the path taken. Look for every write to that name, and check which block each one lives in. 2. **Look for the brace, not the name.** The two declarations often read identically; what differs is that one is inside a nested block. Scanning for `:=` in blocks *below* the declaration you care about is the fastest visual sweep. 3. **Flip `:=` to `=`.** This is the decisive one-character experiment. If the file still compiles, an outer variable of a compatible type was in scope, and the original code was shadowing it. If the compiler now says the variable is undefined, no outer variable existed and you are looking at a different bug. 4. **Write the failing test first.** The bug survives because the failure branch is never exercised. A test that forces the inner call to fail and asserts a non-nil error both reproduces the defect and stops it returning. ## Catching the class The purpose-built tool is the `shadow` analyzer that ships in `golang.org/x/tools/go/analysis/passes/shadow`. Two things about it decide how you use it: - **It is not in `go vet`'s default checks.** A plain `go vet ./...` will not report shadowing; the analyzer has to be installed and run explicitly (its `cmd/shadow` command can be handed to `go vet` through `-vettool`). Teams that assume vet covers it are quietly running with no coverage at all. - **It is deliberately conservative.** It reports an inner declaration only when a variable of the same name *and same type* exists in an outer scope and is referenced *after* the inner declaration — a heuristic chosen because reporting every legal shadow would drown a real codebase in noise. So it catches the shape above (the outer `err` is used by the `return`), and misses variants where the outer variable is only read before the block. Treat it as a net with known holes, not a proof. Expect a first run on an existing codebase to produce a mix of true findings and idiomatic-but-flagged code; triage it once, fix the real ones, and then wire it into CI so the count stays at zero. Adding it to a green build is far cheaper than adding it to a red one. ## Preventing it structurally Tooling is the backstop; the code shapes matter more: - **Declare in the narrowest scope that works.** Most accidental shadows exist because an outer variable was hoisted further out than the code needed. If the value is only interesting inside the block, there is no outer variable to shadow. - **Do not accumulate a result across branches when you can act on it immediately.** The `var err error` at the top of a long loop is the shape most prone to this; handling each failure where it happens removes the outer variable entirely. - **Give different values different names.** When an inner error genuinely is a different thing, `writeErr` or `parseErr` says so, compiles to the same code, and makes the review question disappear. - **Read for it in review.** As the reviewer of a pull request, a `:=` inside a block for a name that also exists outside is worth one question every time. This is one of the few Go defects a reviewer can spot faster than a tool, because the reviewer knows which value is supposed to survive the block. ## What to say about it The honest framing is that shadowing is a feature with a sharp edge, not a language flaw to be eliminated. Inner scopes hiding outer names is what makes small blocks safe to write. The bug is not that Go allows it; it is that the same name is being used for two values with different lifetimes, and the fix — at the instance level and at the codebase level — is to stop doing that where it matters.